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

[Jim Meyering]

Jim Meyering wrote:

> Jim Meyering wrote:
>> I wrote this months ago, and like the minimalist (avoiding unnecessary
>> printf) and more type-safe aspects.  The code-duplication is not ideal,
>> but the duplicated code is small and fragile, and not generally useful
>> enough to put in system.h -- too easy to use it from a context where
>> it would fail.  We could always give it a few more buffer entries, like
>> human.c does, so two invocations in the same statement won't misbehave.
>>
>> Opinions?
>
> No response, so I guess it's not horribly to
> anyone else, either...
>
>> 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         |   38 +++++++++++++++++++++++++++++---------
>>  2 files changed, 41 insertions(+), 13 deletions(-)
> ...
>
> I rebased and was about to push it, but discovered that tests were failing:
> when compiled with -O2 and with today's (from-svn-this-morning) gcc,
>
>     gcc version 4.8.0 20120516 (experimental) (GCC)
>
> it segfaults.  It works fine when compiled with -O1, or with F17's
>
>     gcc (GCC) 4.7.0 20120504 (Red Hat 4.7.0-4)
>
> Whee...  While trying to debug that, I managed to trigger
> a failed assertion in gdb.  First priority is to report that.

Reported here:

    http://sourceware.org/bugzilla/show_bug.cgi?id=14119

> I'll hold off until I know more...

When you write 9 bytes past the end of a stack buffer,
you should expect bad things to happen.  The type used
as the INT_BUFSIZE_BOUND argument must match the *tostr
function that uses that buffer.

Yet another reason I'm glad to use cutting-edge gcc.

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))