Re: stat signed/unsigned

[Pádraig Brady]

I've split up my "avoid -Wsign-compare warnings" patch,
like Jim originally requested.

Attached are IMHO the non contentious patches.

The rest which I'll drop for the moment all boil
down to passing unsigned ints as field widths to
printf, and treating stat.st_size as unsigned.

cheers,
Pádraig.

>From c91df53022c168f84f70364465f58e434794b172 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Wed, 14 Jan 2009 19:17:39 +0000
Subject: [PATCH] simplify mgetgroups() and avoid -Wsign-compare warnings

* gl/lib/mgetgroups.c: Don't explicitly call memcpy() or malloc()
and also avoid -Wsign-compare warning by using appropriate
types for the parameters of realloc_groupbuf().
* src/group-list.c: Use int rather than size_t as variable is
used in signed comparisons.
* src/id.c: ditto.
---
 gl/lib/mgetgroups.c |   46 +++++++++++++++++-----------------------------
 src/group-list.c    |    4 ++--
 src/id.c            |    4 ++--
 3 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/gl/lib/mgetgroups.c b/gl/lib/mgetgroups.c
index ad1fd4f..b31ab89 100644
--- a/gl/lib/mgetgroups.c
+++ b/gl/lib/mgetgroups.c
@@ -1,6 +1,6 @@
 /* mgetgroups.c -- return a list of the groups a user is in
 
-   Copyright (C) 2007-2008 Free Software Foundation, Inc.
+   Copyright (C) 2007-2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -33,16 +33,16 @@
 #include "xalloc.h"
 
 
-static void *
-allocate_groupbuf (int size)
+static GETGROUPS_T *
+realloc_groupbuf (GETGROUPS_T *g, size_t num)
 {
-  if (xalloc_oversized (size, sizeof (GETGROUPS_T)))
+  if (xalloc_oversized (num, sizeof (*g)))
     {
       errno = ENOMEM;
       return NULL;
     }
 
-  return malloc (size * sizeof (GETGROUPS_T));
+  return realloc (g, num * sizeof (*g));
 }
 
 /* Like getugroups, but store the result in malloc'd storage.
@@ -65,45 +65,27 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T 
**groups)
      performance characteristics.
 
      In glibc 2.3.2, getgrouplist is buggy.  If you pass a zero as the
-     size of the output buffer, getgrouplist will still write to the
+     length of the output buffer, getgrouplist will still write to the
      buffer.  Contrary to what some versions of the getgrouplist
      manpage say, this doesn't happen with nonzero buffer sizes.
      Therefore our usage here just avoids a zero sized buffer.  */
   if (username)
     {
       enum { N_GROUPS_INIT = 10 };
-      GETGROUPS_T smallbuf[N_GROUPS_INIT];
-
       max_n_groups = N_GROUPS_INIT;
-      ng = getgrouplist (username, gid, smallbuf, &max_n_groups);
 
-      g = allocate_groupbuf (max_n_groups);
+      g = realloc_groupbuf (NULL, max_n_groups);
       if (g == NULL)
        return -1;
 
-      if (max_n_groups <= N_GROUPS_INIT)
-       {
-         /* smallbuf was big enough, so we already have our data */
-         memcpy (g, smallbuf, max_n_groups * sizeof *g);
-         *groups = g;
-         return max_n_groups;
-       }
-
       while (1)
        {
          GETGROUPS_T *h;
-         ng = getgrouplist (username, gid, g, &max_n_groups);
-         if (0 <= ng)
-           {
-             *groups = g;
-             return ng;
-           }
 
-         /* When getgrouplist fails, it guarantees that
-            max_n_groups reflects the new number of groups.  */
+         /* getgrouplist updates max_n_groups to num required.  */
+         ng = getgrouplist (username, gid, g, &max_n_groups);
 
-         if (xalloc_oversized (max_n_groups, sizeof *h)
-             || (h = realloc (g, max_n_groups * sizeof *h)) == NULL)
+         if ((h = realloc_groupbuf (g, max_n_groups)) == NULL)
            {
              int saved_errno = errno;
              free (g);
@@ -111,6 +93,12 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T 
**groups)
              return -1;
            }
          g = h;
+
+         if (ng >= 0)
+           {
+             *groups = g;
+             return ng;
+           }
        }
     }
   /* else no username, so fall through and use getgroups. */
@@ -125,7 +113,7 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T 
**groups)
   if (max_n_groups < 0)
       max_n_groups = 5;
 
-  g = allocate_groupbuf (max_n_groups);
+  g = realloc_groupbuf (NULL, max_n_groups);
   if (g == NULL)
     return -1;
 
diff --git a/src/group-list.c b/src/group-list.c
index 3547ed6..46895b4 100644
--- a/src/group-list.c
+++ b/src/group-list.c
@@ -1,5 +1,5 @@
 /* group-list.c --Print a list of group IDs or names.
-   Copyright (C) 1989-2008 Free Software Foundation, Inc.
+   Copyright (C) 1989-2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -57,7 +57,7 @@ print_group_list (const char *username,
 #if HAVE_GETGROUPS
   {
     GETGROUPS_T *groups;
-    size_t i;
+    int i;
 
     int n_groups = mgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1),
                                &groups);
diff --git a/src/id.c b/src/id.c
index 156b066..05ad2d8 100644
--- a/src/id.c
+++ b/src/id.c
@@ -1,5 +1,5 @@
 /* id -- print real and effective UIDs and GIDs
-   Copyright (C) 1989-2008 Free Software Foundation, Inc.
+   Copyright (C) 1989-2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -294,7 +294,7 @@ print_full_info (const char *username)
 #if HAVE_GETGROUPS
   {
     GETGROUPS_T *groups;
-    size_t i;
+    int i;
 
     int n_groups = mgetgroups (username, (pwd ? pwd->pw_gid : (gid_t) -1),
                               &groups);
-- 
1.5.3.6


>From 200e218b1520fefeee24538799c71107d63ab082 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 15 Jan 2009 17:26:59 +0000
Subject: [PATCH] shred: Avoid -Wsign-compare warnings

* src/shred.c: Use already assigned signed variable sizeof_r,
rather than the unsigned sizeof(r). Don't use signed integer
overflow check that contemporary compilers may remove anyway.
---
 src/shred.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/shred.c b/src/shred.c
index 1e7bffb..224f671 100644
--- a/src/shred.c
+++ b/src/shred.c
@@ -1,6 +1,6 @@
 /* shred.c - overwrite files and devices to make it harder to recover data
 
-   Copyright (C) 1999-2008 Free Software Foundation, Inc.
+   Copyright (C) 1999-2009 Free Software Foundation, Inc.
    Copyright (C) 1997, 1998, 1999 Colin Plumb.
 
    This program is free software: you can redistribute it and/or modify
@@ -399,7 +399,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type,
   /* Constant fill patterns need only be set up once. */
   if (type >= 0)
     {
-      lim = (0 <= size && size < sizeof_r ? size : sizeof r);
+      lim = (0 <= size && size < sizeof_r ? size : sizeof_r);
       fillpattern (type, r.u, lim);
       passname (r.u, pass_string);
     }
@@ -488,7 +488,7 @@ dopass (int fd, char const *qname, off_t *sizep, int type,
 
       /* Okay, we have written "soff" bytes. */
 
-      if (offset + soff < offset)
+      if (offset > OFF_T_MAX - (off_t) soff)
        {
          error (0, 0, _("%s: file too large"), qname);
          return -1;
-- 
1.5.3.6


>From b71e9294b6217e8505d8c7a9ad15446a3f852bdb Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 15 Jan 2009 17:32:31 +0000
Subject: [PATCH] pr: avoid -Wsign-compare warnings

src/pr.c: Use unsigned variables in unsigned comparisons.
---
 src/pr.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/pr.c b/src/pr.c
index d2b6714..308a025 100644
--- a/src/pr.c
+++ b/src/pr.c
@@ -1,5 +1,5 @@
 /* pr -- convert text files for printing.
-   Copyright (C) 88, 91, 1995-2008 Free Software Foundation, Inc.
+   Copyright (C) 88, 91, 1995-2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -455,7 +455,7 @@ static char *buff;
 
 /* Index of the position in buff where the next character
    will be stored. */
-static int buff_current;
+static unsigned int buff_current;
 
 /* The number of characters in buff.
    Used for allocation of buff and to detect overflow of buff. */
@@ -1944,8 +1944,8 @@ static void
 store_columns (void)
 {
   int i, j;
-  int line = 0;
-  int buff_start;
+  unsigned int line = 0;
+  unsigned int buff_start;
   int last_col;                /* The rightmost column which will be saved in 
buff */
   COLUMN *p;
 
-- 
1.5.3.6


>From b7ea4d0f698cb01815d16a7d7330a3a77c70d807 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
Date: Thu, 15 Jan 2009 17:36:27 +0000
Subject: [PATCH] pathchk: avoid -Wsign-compare warnings

* src/pathchk.c: Compare pathconf limits to signed MAX constants,
as pathconf returns signed values.
---
 src/pathchk.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/pathchk.c b/src/pathchk.c
index 5dbc7da..5261345 100644
--- a/src/pathchk.c
+++ b/src/pathchk.c
@@ -1,5 +1,5 @@
 /* pathchk -- check whether file names are valid or portable
-   Copyright (C) 1991-2008 Free Software Foundation, Inc.
+   Copyright (C) 1991-2009 Free Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -323,7 +323,7 @@ validate_file_name (char *file, bool 
check_basic_portability,
                     dir);
              return false;
            }
-         maxsize = MIN (size, SIZE_MAX);
+         maxsize = MIN (size, SSIZE_MAX);
        }
 
       if (maxsize <= filelen)
@@ -385,7 +385,7 @@ validate_file_name (char *file, bool 
check_basic_portability,
              len = pathconf (dir, _PC_NAME_MAX);
              *start = c;
              if (0 <= len)
-               name_max = MIN (len, SIZE_MAX);
+               name_max = MIN (len, SSIZE_MAX);
              else
                switch (errno)
                  {
-- 
1.5.3.6