GNOME Bugzilla – Bug 541945
[PATCH] Parse gecos field (i.e. don't show username commas)
Last modified: 2009-05-26 19:52:50 UTC
The authentication dialog shows user names pulled from a passwd struct returned by getpwnam. The relevant field is pw_gecos, and currently the code uses the full string. However, according to the man page (e.g. http://www.daemon-systems.org/man/passwd.5.html), pw_gecos is a comma-delimited field containing first, the user's name, and then data like various phone numbers. I'd argue that we only want to show the first field of that comma-delimited list. In the common case, we merely get several unsightly commas. But if there's actual data there, the combo box would look terrible. Attached is a patch to parse pw_gecos. The only issue is whether normal commas can appear in a user's name. /usr/sbin/adduser didn't let me use one. Presumably you can't? Copyright is Canonical, Ltd.'s.
Created attachment 114142 [details] [review] Proposed patch
Created attachment 114144 [details] [review] Patch v2 Whoops. I realized once I hit 'Submit' that I neglected to handle the case of the first delimited field being empty. Here ya go.
Patch looks fine to me from a cursory look except for coding standard issues (don't treat pointers as booleans in the if statement, declaration/assignment should be on separate lines). However, please note that - All this code has basically been rewritten for PolicKit version 0.90 and up - GNOME 2.27/2.28 will switch to this new version of PolicyKit-gnome - PolicyKit-gnome is in git, see http://git.gnome.org/cgit/PolicyKit-gnome/ I think the PolicyKit in git HEAD still suffers from that so it would be good to fix that.
*** Bug 557478 has been marked as a duplicate of this bug. ***
Created attachment 129416 [details] [review] Patch against git So this patch is against git master. It changes one thing besides style changes you suggested: I grabbed the g_locale_to_utf8 call from tools/polkit-gnome-authorization.c. That file had similar code, but was using g_locale_to_utf8. While I assumed that code was dead (tools isn't a SUBDIR in Makefile.am), I also assumed that that call had been there for a reason. If that code isn't dead, I can patch it too. man 5 passwd didn't give any info about the expected encoding. If I'm wrong and we don't need g_locale_to_utf8 there, just replace it with a g_strdup, please.
Fixed in commit e758975e8b429f5aa1d80cd9e928236ddcf9380f. Thanks!