GNOME Bugzilla – Bug 338916
Missing constants, implementing FILE_INFO_ACCESS and a typo
Last modified: 2006-05-01 15:32:22 UTC
Please describe the problem: 1. The gnomevfs module doesn't expose the FILE_INFO_FIELDS_ACCESS, FILE_INFO_FIELDS_IDS and PERM_ACCESS_* flags, and a couple other ones 2. When setting uid and gid in FileInfo, it's incorrectly toggling the FILE_INFO_FIELDS_PERMISSIONS flag in valid_flags. The correct one is FILE_INFO_FIELDS_IDS 3. When 'gid' is not set the error message incorrectly mentioned 'uid' Steps to reproduce: 1. It was impossible to check for PERM_ACCESS_* without this. 2. Some applications (eg: nautilus) would not be able to show uid/gid because of the missing FILE_INFO_FIELDS_IDS toggle in valid_fields 3. Just a typo Actual results: Expected results: Does this happen every time? Other information:
Created attachment 63807 [details] [review] Proposed patch for the bug
Created attachment 63808 [details] [review] Proposed patch for the bug (updated)
For the records, the motivation for working on this issue was to make gedit work with a pythonmethod-based vfs. However only after submitting the patch I realized a additional step was required: to add the pythonmethod-based scheme to '/apps/gedit-2/preferences/editor/save/writable_vfs_schemes'. Happily, with the attached patch, and the scheme added to gedit's gconf prefs, gedit will properly check for PREFS_ACCESS_WRITABLE before marking the file readonly.
For reference, here's the code from gedit that does the check: ... if (loader->priv->info && (loader->priv->info->valid_fields & GNOME_VFS_FILE_INFO_FIELDS_ACCESS)) return (loader->priv->info->permissions & GNOME_VFS_PERM_ACCESS_WRITABLE) ? FALSE : TRUE; ...
Damn, I missed this one on gnome-python 2.15.1 :(
Comment on attachment 63808 [details] [review] Proposed patch for the bug (updated) >@@ -239,8 +240,18 @@ > "'permissions' attribute must be an int"); > return -1; > } >- finfo->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_PERMISSIONS; > finfo->permissions = PyInt_AsLong(value); >+ /* Always set the permissions field, no matter what */ >+ finfo->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_PERMISSIONS; >+ /* But only set the access bit if access is being set */ >+ if ((finfo->permissions & GNOME_VFS_PERM_ACCESS_READABLE) || >+ (finfo->permissions & GNOME_VFS_PERM_ACCESS_WRITABLE) || >+ (finfo->permissions & GNOME_VFS_PERM_ACCESS_EXECUTABLE)) { >+ finfo->valid_fields |= GNOME_VFS_FILE_INFO_FIELDS_ACCESS; >+ } >+ else { >+ finfo->valid_fields ^= GNOME_VFS_FILE_INFO_FIELDS_ACCESS; This is the only part I'm not sure about. You know that ^= means exclusive OR, right? That line simply toggles the value of the GNOME_VFS_FILE_INFO_FIELDS_ACCESS bit. If it was zero before, it becomes one. Maybe you meant something like this? finfo->valid_fields &= ~GNOME_VFS_FILE_INFO_FIELDS_ACCESS; But there is a deeper problem. It is not correct to set GNOME_VFS_FILE_INFO_FIELDS_ACCESS only when any of the ACCESS bits is set to one. GNOME_VFS_FILE_INFO_FIELDS_ACCESS should be set to indicate the access bits have been _filled_ by the programmer, either to one or zero, doesn't matter (you could want to tell GnomeVFS there should be no access to a file). We usually do so on the respective setter. Unfortunately the access bits are in the same field as the permission bits, so it is effectively impossible to distinguish them. The only possible way is to create a fake 'access' field which, when written, writes to ->permissions and sets the GNOME_VFS_FILE_INFO_FIELDS_ACCESS flag accordingly. But I'll fix the patch and commit, don't worry.