Re: [PATCH] id,groups: use gidtostr/uidtostr to avoid casts

[Jim Meyering]

Jim Meyering wrote:
...
>> I'm squashing this in:
>>
>> diff --git a/src/id.c b/src/id.c
>> index d060a94..1f99b9f 100644
>> --- a/src/id.c
>> +++ b/src/id.c
>> @@ -280,7 +280,7 @@ main (int argc, char **argv)
>>  static char *
>>  gidtostr_ptr (gid_t const *gid)
>>  {
>> -  static char buf[INT_BUFSIZE_BOUND (gid_t)];
>> +  static char buf[INT_BUFSIZE_BOUND (uintmax_t)];
>>    return umaxtostr (*gid, buf);
>>  }
>>  #define gidtostr(g) gidtostr_ptr (&(g))
>> @@ -291,7 +291,7 @@ gidtostr_ptr (gid_t const *gid)
>>  static char *
>>  uidtostr_ptr (uid_t const *uid)
>>  {
>> -  static char buf[INT_BUFSIZE_BOUND (uid_t)];
>> +  static char buf[INT_BUFSIZE_BOUND (uintmax_t)];
>>    return umaxtostr (*uid, buf);
>>  }
>>  #define uidtostr(u) uidtostr_ptr (&(u))

That was the bug fix for a not-yet-pushed patch.

> And the same for the duplicated function (hmm... :-( )
> in group-list.c

Since then, I spotted two more places in id.c that can trivially use
the new gidtostr function instead of a cast and %lu conversion,
so I'm going ahead with this:

>From b663bc8eb0b87c21584b931872db5fd82cf72f49 Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sun, 1 Jan 2012 22:31:41 +0100
Subject: [PATCH] id,groups: use gidtostr/uidtostr to avoid casts

* src/id.c (gidtostr, uidtostr): Define macros.
(gidtostr_ptr, uidtostr_ptr): Define safer functions.
Use gidtostr and uidtostr to print GID and UID without
need/risk of casts.
* src/group-list.c: Likewise.
---
 src/group-list.c | 16 ++++++++++++----
 src/id.c         | 42 +++++++++++++++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/src/group-list.c b/src/group-list.c
index edbb342..a25601e 100644
--- a/src/group-list.c
+++ b/src/group-list.c
@@ -88,6 +88,16 @@ print_group_list (const char *username,
   return ok;
 }

+/* Convert a gid_t to string.  Do not use this function directly.
+   Instead, use it via the gidtostr macro.
+   Beware that it returns a pointer to static storage.  */
+static char *
+gidtostr_ptr (gid_t const *gid)
+{
+  static char buf[INT_BUFSIZE_BOUND (uintmax_t)];
+  return umaxtostr (*gid, buf);
+}
+#define gidtostr(g) gidtostr_ptr (&(g))

 /* Print the name or value of group ID GID. */
 extern bool
@@ -107,9 +117,7 @@ print_group (gid_t gid, bool use_name)
         }
     }

-  if (grp == NULL)
-    printf ("%lu", (unsigned long int) gid);
-  else
-    printf ("%s", grp->gr_name);
+  char *s = grp ? grp->gr_name : gidtostr (gid);
+  fputs (s, stdout);
   return ok;
 }
diff --git a/src/id.c b/src/id.c
index 41ae024..435d9e8 100644
--- a/src/id.c
+++ b/src/id.c
@@ -274,6 +274,28 @@ main (int argc, char **argv)
   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }

+/* Convert a gid_t to string.  Do not use this function directly.
+   Instead, use it via the gidtostr macro.
+   Beware that it returns a pointer to static storage.  */
+static char *
+gidtostr_ptr (gid_t const *gid)
+{
+  static char buf[INT_BUFSIZE_BOUND (uintmax_t)];
+  return umaxtostr (*gid, buf);
+}
+#define gidtostr(g) gidtostr_ptr (&(g))
+
+/* Convert a uid_t to string.  Do not use this function directly.
+   Instead, use it via the uidtostr macro.
+   Beware that it returns a pointer to static storage.  */
+static char *
+uidtostr_ptr (uid_t const *uid)
+{
+  static char buf[INT_BUFSIZE_BOUND (uintmax_t)];
+  return umaxtostr (*uid, buf);
+}
+#define uidtostr(u) uidtostr_ptr (&(u))
+
 /* Print the name or value of user ID UID. */

 static void
@@ -286,16 +308,14 @@ print_user (uid_t uid)
       pwd = getpwuid (uid);
       if (pwd == NULL)
         {
-          error (0, 0, _("cannot find name for user ID %lu"),
-                 (unsigned long int) uid);
+          error (0, 0, _("cannot find name for user ID %s"),
+                 uidtostr (uid));
           ok = false;
         }
     }

-  if (pwd == NULL)
-    printf ("%lu", (unsigned long int) uid);
-  else
-    printf ("%s", pwd->pw_name);
+  char *s = pwd ? pwd->pw_name : uidtostr (uid);
+  fputs (s, stdout);
 }

 /* Print all of the info about the user's user and group IDs. */
@@ -306,19 +326,19 @@ print_full_info (const char *username)
   struct passwd *pwd;
   struct group *grp;

-  printf (_("uid=%lu"), (unsigned long int) ruid);
+  printf (_("uid=%s"), uidtostr (ruid));
   pwd = getpwuid (ruid);
   if (pwd)
     printf ("(%s)", pwd->pw_name);

-  printf (_(" gid=%lu"), (unsigned long int) rgid);
+  printf (_(" gid=%s"), gidtostr (rgid));
   grp = getgrgid (rgid);
   if (grp)
     printf ("(%s)", grp->gr_name);

   if (euid != ruid)
     {
-      printf (_(" euid=%lu"), (unsigned long int) euid);
+      printf (_(" euid=%s"), uidtostr (euid));
       pwd = getpwuid (euid);
       if (pwd)
         printf ("(%s)", pwd->pw_name);
@@ -326,7 +346,7 @@ print_full_info (const char *username)

   if (egid != rgid)
     {
-      printf (_(" egid=%lu"), (unsigned long int) egid);
+      printf (_(" egid=%s"), gidtostr (egid));
       grp = getgrgid (egid);
       if (grp)
         printf ("(%s)", grp->gr_name);
@@ -359,7 +379,7 @@ print_full_info (const char *username)
       {
         if (i > 0)
           putchar (',');
-        printf ("%lu", (unsigned long int) groups[i]);
+        fputs (gidtostr (groups[i]), stdout);
         grp = getgrgid (groups[i]);
         if (grp)
           printf ("(%s)", grp->gr_name);
--
1.7.10.2.552.gaa3bb87