GNOME Bugzilla – Bug 433898
Set perms on ~/.face and cleanup
Last modified: 2007-04-30 17:35:37 UTC
Sometimes, the ~/.face set by about-me won't be used by gdm, because gdm is anal about the permissions of the file. See https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=236393#c10 for details The attached patch cleans up the code a bit (fwrite without retval checking is gross), and sets the permissions on the file after it's been created, whatever the default umask, so that gdm can use it.
Created attachment 87151 [details] [review] control-center-trunk-set-face-perms.patch
Shouldn't you check the return codes of the unlink and set_contents calls as well? Also, why 755? I would have expected 644. From a quick look at the gdm code, that should work as well.
(In reply to comment #2) > Shouldn't you check the return codes of the unlink and set_contents calls as > well? To do what? We could certainly check for errors, and print a warning on the command-line. > Also, why 755? I would have expected 644. From a quick look at the gdm code, > that should work as well. Agreed. It's just that the default perms for that file was 775 when created, on a default install. 644 is fine by me as well :)
> To do what? We could certainly check for errors, and print a warning on the > command-line. I admit it probably isn't terribly important, but if e.g. removing the file failed, trying to rewrite it or change its permissions doesn't make a lot of sense.
Created attachment 87270 [details] [review] control-center-trunk-set-face-perms-3.patch Fix build warnings, and print an untranslated error on the command-line if there's a problem with creating the ~/.face
Hm, do we actually need the g_unlink()? Apart from that, I'll still insist on 644. Otherwise ok.
Created attachment 87277 [details] [review] control-center-trunk-set-face-perms-4.patch Use 0644 as the mode, don't use g_unlink
Fixed in trunk, should I also commit to gnome-2-18? 2007-04-30 Bastien Nocera <hadess@hadess.net> * gnome-about-me.c: (about_me_update_photo): Use g_file_set_contents instead of a single fwrite to write the ~/.face image, also set the default permission to be 0644, even if the umask is more permissive, otherwise GDM won't show the icon in the chooser (Closes: #433898)
(In reply to comment #8) > Fixed in trunk, should I also commit to gnome-2-18? I'd say it's not serious enough, so unless somebody disagrees...