bug-coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

coreutils fts + du fixes and cleanup


From: Paul Eggert
Subject: coreutils fts + du fixes and cleanup
Date: Mon, 02 Aug 2004 12:51:25 -0700
User-agent: Gnus/5.1006 (Gnus v5.10.6) Emacs/21.3 (gnu/linux)

This fixes some bugs with fts and/or du when they are run on
filesystems whose recursion depth exceeds INT_MAX, or containing file
names whose lengths are longer than 65536.  The former is possible
(though tedious to create :-) on many modern filesystems and will
cause bugs in "du" and other programs on 64-bit hosts; the latter is
possible on systems like GNU/Hurd that don't impose arbitrary limits
on file name lengths.

fts in coreutils has diverged from fts in glibc, partly due to bug
fixes and partly due to the greater robustness requirements of
coreutils.  No doubt fixing these problems in glibc will be a bit of
work, due to backward-compatibility constraints, but someone should
undertake it at some point.

This change also adds an FSF copyright notice to fts; it's time, given
all the changes.

2004-08-02  Paul Eggert  <address@hidden>

        * lib/fts_.h: Add an FSF copyright notice, since our changes are
        becoming nontrivial.
        * lib/fts.c: Likewise.
        * lib/fts_.h: Include stddef.h, for ptrdiff_t.
        (FTS.fts_nitems): Now size_t, not int, for hosts that allow more
        than INT_MAX entries in a directory.
        (FTS_ROOTPARENTLEVEL): Parenthesize properly.
        (FTSENT.fts_level): Now ptrdiff_t, not int, to allow recursing more
        than INT_MAX levels deep on 64-bit hosts.
        (FTSENT.fts_namelen): Now size_t, not u_short, to support hosts like
        the Hurd that don't have arbitrary limits on directory entry lengths.
        (FTSENT.fts_statp): Now an array, not a pointer, so that we don't
        have to play unportable games with pointer arithmetic.  Keep it array
        for the benefit of user code that assumes it is a pointer.
        * lib/fts.c: Include stdint.h if available, as Autoconf suggests.
        (ALIGNBYTES, ALIGN): Remove; no longer needed now that fts_statp
        is an array.
        (fts_alloc, fts_palloc, fts_sort, fts_load, fts_build):
        Use size_t for sizes.
        (fts_stat, fts_safe_changedir, fts_debug, fts_read, fts_build,
        fts_palloc):
        Use bool when appropriate.
        (SIZE_MAX, TYPE_SIGNED): New macros.
        (fts_read): Use u_short for instructions.
        (fts_build): Use ptrdiff_t for levels.  Don't assume file name lengths
        fit into int.  Don't assume nlink_t is signed.
        (find_matching_ancestor): Don't assume dev, ino fit in int.
        (fts_stat): Use function prototype; required for bool arg.
        (fts_sort): Detect integer overflow in size calculations.
        (fts_alloc): Simplify allocation code, now that fts_statp is an array
        and not a pointer.

Index: lib/fts_.h
===================================================================
RCS file: /home/eggert/coreutils/cu/lib/fts_.h,v
retrieving revision 1.10
diff -p -u -r1.10 fts_.h
--- lib/fts_.h  19 Dec 2003 12:40:05 -0000      1.10
+++ lib/fts_.h  2 Aug 2004 19:39:13 -0000
@@ -1,3 +1,21 @@
+/* Traverse a file hierarchy.
+
+   Copyright (C) 2004 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
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software Foundation,
+   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+
 /*
  * Copyright (c) 1989, 1993
  *     The Regents of the University of California.  All rights reserved.
@@ -43,6 +61,7 @@
 #  define __END_DECLS
 # endif
 
+# include <stddef.h>
 # include <sys/types.h>
 # include "hash.h"
 # include "cycle-check.h"
@@ -55,7 +74,7 @@ typedef struct {
        char *fts_path;                 /* path for this descent */
        int fts_rfd;                    /* fd for root */
        size_t fts_pathlen;             /* sizeof(path) */
-       int fts_nitems;                 /* elements in the sort array */
+       size_t fts_nitems;                      /* elements in the sort array */
        int (*fts_compar) (const void *, const void *); /* compare fn */
 
 # define FTS_COMFOLLOW 0x0001          /* follow command line symlinks */
@@ -116,11 +135,11 @@ typedef struct _ftsent {
        dev_t fts_dev;                  /* device */
        nlink_t fts_nlink;              /* link count */
 
-# define FTS_ROOTPARENTLEVEL   -1
+# define FTS_ROOTPARENTLEVEL   (-1)
 # define FTS_ROOTLEVEL          0
-       int fts_level;                  /* depth (-1 to N) */
+       ptrdiff_t fts_level;            /* depth (-1 to N) */
 
-       u_short fts_namelen;            /* strlen(fts_name) */
+       size_t fts_namelen;             /* strlen(fts_name) */
 
 # define FTS_D          1              /* preorder directory */
 # define FTS_DC                 2              /* directory that causes cycles 
*/
@@ -148,7 +167,7 @@ typedef struct _ftsent {
 # define FTS_SKIP       4              /* discard node */
        u_short fts_instr;              /* fts_set() instructions */
 
-       struct stat *fts_statp;         /* stat(2) information */
+       struct stat fts_statp[1];       /* stat(2) information */
        char fts_name[1];               /* file name */
 } FTSENT;
 
Index: lib/fts.c
===================================================================
RCS file: /home/eggert/coreutils/cu/lib/fts.c,v
retrieving revision 1.15
diff -p -u -r1.15 fts.c
--- lib/fts.c   8 Jun 2004 13:31:43 -0000       1.15
+++ lib/fts.c   2 Aug 2004 19:39:22 -0000
@@ -1,3 +1,21 @@
+/* Traverse a file hierarchy.
+
+   Copyright (C) 2004 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
+   the Free Software Foundation; either version 2, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, write to the Free Software Foundation,
+   Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.  */
+
 /*-
  * Copyright (c) 1990, 1993, 1994
  *     The Regents of the University of California.  All rights reserved.
@@ -55,6 +73,9 @@ static char sccsid[] = "@(#)fts.c     8.6 (B
 #if HAVE_INTTYPES_H
 # include <inttypes.h>
 #endif
+#if HAVE_STDINT_H
+# include <stdint.h>
+#endif
 
 #if defined _LIBC
 # include <dirent.h>
@@ -109,28 +130,16 @@ int rpl_lstat (const char *, struct stat
 # define __set_errno(Val) errno = (Val)
 #endif
 
-/* Largest alignment size needed, minus one.
-   Usually long double is the worst case.  */
-# if __GNUC__ >= 2
-# define ALIGNBYTES    (__alignof__ (long double) - 1)
-#else
-# define ALIGNBYTES    (sizeof (long double) - 1)
-#endif
-/* Align P to that size.  */
-#ifndef ALIGN
-# define ALIGN(p)      (((unsigned long int) (p) + ALIGNBYTES) & ~ALIGNBYTES)
-#endif
 
-
-static FTSENT  *fts_alloc __P((FTS *, const char *, int)) internal_function;
+static FTSENT  *fts_alloc __P((FTS *, const char *, size_t)) internal_function;
 static FTSENT  *fts_build __P((FTS *, int)) internal_function;
 static void     fts_lfree __P((FTSENT *)) internal_function;
 static void     fts_load __P((FTS *, FTSENT *)) internal_function;
 static size_t   fts_maxarglen __P((char * const *)) internal_function;
 static void     fts_padjust __P((FTS *, FTSENT *)) internal_function;
-static int      fts_palloc __P((FTS *, size_t)) internal_function;
-static FTSENT  *fts_sort __P((FTS *, FTSENT *, int)) internal_function;
-static u_short  fts_stat __P((FTS *, FTSENT *, int)) internal_function;
+static bool     fts_palloc __P((FTS *, size_t)) internal_function;
+static FTSENT  *fts_sort __P((FTS *, FTSENT *, size_t)) internal_function;
+static u_short  fts_stat __P((FTS *, FTSENT *, bool)) internal_function;
 static int      fts_safe_changedir __P((FTS *, FTSENT *, int, const char *))
      internal_function;
 
@@ -138,6 +147,13 @@ static int      fts_safe_changedir __P((
 # define MAX(a,b) ((a) > (b) ? (a) : (b))
 #endif
 
+#ifndef SIZE_MAX
+# define SIZE_MAX ((size_t) -1)
+#endif
+
+/* The extra casts work around common compiler bugs.  */
+#define TYPE_SIGNED(t) (! ((t) 0 < (t) -1))
+
 #define ISDOT(a)       (a[0] == '.' && (!a[1] || (a[1] == '.' && !a[2])))
 
 #define CLR(opt)       (sp->fts_options &= ~(opt))
@@ -154,7 +170,7 @@ static int      fts_safe_changedir __P((
 #define HT_INITIAL_SIZE 31
 
 #if FTS_DEBUG
-int fts_debug = 0;
+bool fts_debug = false;
 # include <stdio.h>
 # define Dprintf(x) do { if (fts_debug) printf x; } while (0)
 #else
@@ -278,9 +294,9 @@ fts_open(argv, options, compar)
 {
        register FTS *sp;
        register FTSENT *p, *root;
-       register int nitems;
+       register size_t nitems;
        FTSENT *parent, *tmp = NULL;    /* pacify gcc */
-       int len;
+       size_t len;
 
        /* Options check. */
        if (options & ~FTS_OPTIONMASK) {
@@ -306,7 +322,7 @@ fts_open(argv, options, compar)
 #ifndef MAXPATHLEN
 # define MAXPATHLEN 1024
 #endif
-       if (fts_palloc(sp, MAX(fts_maxarglen(argv), MAXPATHLEN)))
+       if (! fts_palloc(sp, MAX(fts_maxarglen(argv), MAXPATHLEN)))
                goto mem1;
 
        /* Allocate/initialize root's parent. */
@@ -327,7 +343,7 @@ fts_open(argv, options, compar)
                p->fts_level = FTS_ROOTLEVEL;
                p->fts_parent = parent;
                p->fts_accpath = p->fts_name;
-               p->fts_info = fts_stat(sp, p, ISSET(FTS_COMFOLLOW));
+               p->fts_info = fts_stat(sp, p, ISSET(FTS_COMFOLLOW) != 0);
 
                /* Command-line "." and ".." are real directories. */
                if (p->fts_info == FTS_DOT)
@@ -407,7 +423,7 @@ fts_load(sp, p)
        FTS *sp;
        register FTSENT *p;
 {
-       register int len;
+       register size_t len;
        register char *cp;
 
        /*
@@ -494,7 +510,7 @@ fts_read(sp)
        register FTS *sp;
 {
        register FTSENT *p, *tmp;
-       register int instr;
+       register u_short instr;
        register char *t;
        int saved_errno;
 
@@ -511,7 +527,7 @@ fts_read(sp)
 
        /* Any type of file may be re-visited; re-stat and re-turn. */
        if (instr == FTS_AGAIN) {
-               p->fts_info = fts_stat(sp, p, 0);
+               p->fts_info = fts_stat(sp, p, false);
                return (p);
        }
        Dprintf (("fts_read: p=%s\n",
@@ -525,7 +541,7 @@ fts_read(sp)
         */
        if (instr == FTS_FOLLOW &&
            (p->fts_info == FTS_SL || p->fts_info == FTS_SLNONE)) {
-               p->fts_info = fts_stat(sp, p, 1);
+               p->fts_info = fts_stat(sp, p, true);
                if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) {
                        if ((p->fts_symfd = open(".", O_RDONLY, 0)) < 0) {
                                p->fts_errno = errno;
@@ -627,7 +643,7 @@ next:       tmp = p;
                if (p->fts_instr == FTS_SKIP)
                        goto next;
                if (p->fts_instr == FTS_FOLLOW) {
-                       p->fts_info = fts_stat(sp, p, 1);
+                       p->fts_info = fts_stat(sp, p, true);
                        if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) {
                                if ((p->fts_symfd =
                                    open(".", O_RDONLY, 0)) < 0) {
@@ -808,12 +824,17 @@ fts_build(sp, type)
 {
        register struct dirent *dp;
        register FTSENT *p, *head;
-       register int nitems;
+       register size_t nitems;
        FTSENT *cur, *tail;
        DIR *dirp;
        void *oldaddr;
-       int cderrno, descend, level, nlinks, saved_errno,
-           nostat, doadjust;
+       int cderrno;
+       int saved_errno;
+       bool descend;
+       bool doadjust;
+       ptrdiff_t level;
+       nlink_t nlinks;
+       bool nostat;
        size_t len, maxlen, new_len;
        char *cp;
 
@@ -843,25 +864,20 @@ fts_build(sp, type)
        /*
         * Nlinks is the number of possible entries of type directory in the
         * directory if we're cheating on stat calls, 0 if we're not doing
-        * any stat calls at all, -1 if we're doing stats on everything.
+        * any stat calls at all, (nlink_t) -1 if we're statting everything.
         */
        if (type == BNAMES) {
                nlinks = 0;
                /* Be quiet about nostat, GCC. */
-               nostat = 0;
+               nostat = false;
        } else if (ISSET(FTS_NOSTAT) && ISSET(FTS_PHYSICAL)) {
                nlinks = cur->fts_nlink - (ISSET(FTS_SEEDOT) ? 0 : 2);
-               nostat = 1;
+               nostat = true;
        } else {
                nlinks = -1;
-               nostat = 0;
+               nostat = false;
        }
 
-#ifdef notdef
-       (void)printf("nlinks == %d (cur: %d)\n", nlinks, cur->fts_nlink);
-       (void)printf("NOSTAT %d PHYSICAL %d SEEDOT %d\n",
-           ISSET(FTS_NOSTAT), ISSET(FTS_PHYSICAL), ISSET(FTS_SEEDOT));
-#endif
        /*
         * If we're going to need to stat anything or we want to descend
         * and stay in the directory, chdir.  If this fails we keep going,
@@ -883,14 +899,14 @@ fts_build(sp, type)
                        if (nlinks && type == BREAD)
                                cur->fts_errno = errno;
                        cur->fts_flags |= FTS_DONTCHDIR;
-                       descend = 0;
+                       descend = false;
                        cderrno = errno;
                        (void)closedir(dirp);
                        dirp = NULL;
                } else
-                       descend = 1;
+                       descend = true;
        } else
-               descend = 0;
+               descend = false;
 
        /*
         * Figure out the max file name length that can be stored in the
@@ -916,16 +932,16 @@ fts_build(sp, type)
        level = cur->fts_level + 1;
 
        /* Read the directory, attaching each entry to the `link' pointer. */
-       doadjust = 0;
+       doadjust = false;
        for (head = tail = NULL, nitems = 0; dirp && (dp = readdir(dirp));) {
                if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
                        continue;
 
-               if ((p = fts_alloc(sp, dp->d_name, (int)NAMLEN (dp))) == NULL)
+               if ((p = fts_alloc(sp, dp->d_name, NAMLEN (dp))) == NULL)
                        goto mem1;
                if (NAMLEN (dp) >= maxlen) {/* include space for NUL */
                        oldaddr = sp->fts_path;
-                       if (fts_palloc(sp, NAMLEN (dp) + len + 1)) {
+                       if (! fts_palloc(sp, NAMLEN (dp) + len + 1)) {
                                /*
                                 * No more memory for path or structures.  Save
                                 * errno, free up the current structure and the
@@ -943,7 +959,7 @@ mem1:                               saved_errno = errno;
                        }
                        /* Did realloc() change the pointer? */
                        if (oldaddr != sp->fts_path) {
-                               doadjust = 1;
+                               doadjust = true;
                                if (ISSET(FTS_NOCHDIR))
                                        cp = sp->fts_path + len;
                        }
@@ -999,10 +1015,12 @@ mem1:                            saved_errno = errno;
                        } else
                                p->fts_accpath = p->fts_name;
                        /* Stat it. */
-                       p->fts_info = fts_stat(sp, p, 0);
+                       p->fts_info = fts_stat(sp, p, false);
 
                        /* Decrement link count if applicable. */
-                       if (nlinks > 0 && (p->fts_info == FTS_D ||
+                       if (nlinks > 0
+                           && (TYPE_SIGNED (nlink_t) || nostat)
+                           && (p->fts_info == FTS_D ||
                            p->fts_info == FTS_DC || p->fts_info == FTS_DOT))
                                --nlinks;
                }
@@ -1085,13 +1103,13 @@ find_matching_ancestor (FTSENT const *e_
   printf ("active dirs:\n");
   for (ent = e_curr;
        ent->fts_level >= FTS_ROOTLEVEL; ent = ent->fts_parent)
-    printf ("  %s(%d/%d) to %s(%d/%d)...\n",
+    printf ("  %s(%lu/%lu) to %s(%lu/%lu)...\n",
            ad->fts_ent->fts_accpath,
-           (int)ad->dev,
-           (int)ad->ino,
+           (unsigned long int) ad->dev,
+           (unsigned long int) ad->ino,
            ent->fts_accpath,
-           (int)ent->fts_statp->st_dev,
-           (int)ent->fts_statp->st_ino);
+           (unsigned long int) ent->fts_statp->st_dev,
+           (unsigned long int) ent->fts_statp->st_ino);
 }
 
 void
@@ -1131,10 +1149,7 @@ fts_cross_check (FTS const *sp)
 
 static u_short
 internal_function
-fts_stat(sp, p, follow)
-       FTS *sp;
-       register FTSENT *p;
-       int follow;
+fts_stat(FTS *sp, register FTSENT *p, bool follow)
 {
        struct stat *sbp = p->fts_statp;
        int saved_errno;
@@ -1198,7 +1213,7 @@ internal_function
 fts_sort(sp, head, nitems)
        FTS *sp;
        FTSENT *head;
-       register int nitems;
+       register size_t nitems;
 {
        register FTSENT **ap, *p;
 
@@ -1213,8 +1228,9 @@ fts_sort(sp, head, nitems)
                struct _ftsent **a;
 
                sp->fts_nitems = nitems + 40;
-               if ((a = realloc(sp->fts_array,
-                   (size_t)(sp->fts_nitems * sizeof(FTSENT *)))) == NULL) {
+               if (SIZE_MAX / sizeof *a < sp->fts_nitems
+                   || ! (a = realloc (sp->fts_array,
+                                      sp->fts_nitems * sizeof *a))) {
                        free(sp->fts_array);
                        sp->fts_array = NULL;
                        sp->fts_nitems = 0;
@@ -1236,20 +1252,16 @@ internal_function
 fts_alloc(sp, name, namelen)
        FTS *sp;
        const char *name;
-       register int namelen;
+       register size_t namelen;
 {
        register FTSENT *p;
        size_t len;
 
        /*
         * The file name is a variable length array.  Allocate the FTSENT
-        * structure, the file name and the stat structure in one chunk, but
-        * be careful that the stat structure is reasonably aligned.  Since the
-        * fts_name field is declared to be of size 1, the fts_name pointer is
-        * namelen + 2 before the first possible address of the stat structure.
+        * structure and the file name in one chunk.
         */
        len = sizeof(FTSENT) + namelen;
-       len += sizeof(struct stat) + ALIGNBYTES;
        if ((p = malloc(len)) == NULL)
                return (NULL);
 
@@ -1257,7 +1269,6 @@ fts_alloc(sp, name, namelen)
        memmove(p->fts_name, name, namelen);
        p->fts_name[namelen] = '\0';
 
-       p->fts_statp = (struct stat *)ALIGN(p->fts_name + namelen + 2);
        p->fts_namelen = namelen;
        p->fts_path = sp->fts_path;
        p->fts_errno = 0;
@@ -1288,7 +1299,7 @@ fts_lfree(head)
  * though the kernel won't resolve them.  Add the size (not just what's needed)
  * plus 256 bytes so don't realloc the path 2 bytes at a time.
  */
-static int
+static bool
 internal_function
 fts_palloc(sp, more)
        FTS *sp;
@@ -1307,17 +1318,17 @@ fts_palloc(sp, more)
                }
                sp->fts_path = NULL;
                __set_errno (ENAMETOOLONG);
-               return (1);
+               return false;
        }
        sp->fts_pathlen = new_len;
        p = realloc(sp->fts_path, sp->fts_pathlen);
        if (p == NULL) {
                free(sp->fts_path);
                sp->fts_path = NULL;
-               return 1;
+               return false;
        }
        sp->fts_path = p;
-       return 0;
+       return true;
 }
 
 /*
Index: src/du.c
===================================================================
RCS file: /home/eggert/coreutils/cu/src/du.c,v
retrieving revision 1.197
diff -p -u -r1.197 du.c
--- src/du.c    30 Jun 2004 22:31:43 -0000      1.197
+++ src/du.c    17 Jul 2004 05:07:42 -0000
@@ -43,7 +43,7 @@
 #include "xfts.h"
 #include "xstrtol.h"
 
-extern int fts_debug;
+extern bool fts_debug;
 
 /* The official name of this program (e.g., no `g' prefix).  */
 #define PROGRAM_NAME "du"
@@ -77,29 +77,29 @@ static Hash_table *htab;
 /* Name under which this program was invoked.  */
 char *program_name;
 
-/* If nonzero, display counts for all files, not just directories.  */
-static int opt_all = 0;
+/* If true, display counts for all files, not just directories.  */
+static bool opt_all = false;
 
-/* If nonzero, rather than using the disk usage of each file,
+/* If true, rather than using the disk usage of each file,
    use the apparent size (a la stat.st_size).  */
-static int apparent_size = 0;
+static bool apparent_size = false;
 
-/* If nonzero, count each hard link of files with multiple links.  */
-static int opt_count_all = 0;
+/* If true, count each hard link of files with multiple links.  */
+static bool opt_count_all = false;
 
 /* If true, output the NUL byte instead of a newline at the end of each line. 
*/
 static bool opt_nul_terminate_output = false;
 
-/* If nonzero, print a grand total at the end.  */
-static int print_grand_total = 0;
+/* If true, print a grand total at the end.  */
+static bool print_grand_total = false;
 
 /* If nonzero, do not add sizes of subdirectories.  */
-static int opt_separate_dirs = 0;
+static bool opt_separate_dirs = false;
 
 /* Show the total for each directory (and file if --all) that is at
    most MAX_DEPTH levels down from the root of the hierarchy.  The root
    is at level 0, so `du --max-depth=0' is equivalent to `du -s'.  */
-static int max_depth = INT_MAX;
+static size_t max_depth = SIZE_MAX;
 
 /* Human-readable options for output.  */
 static int human_output_opts;
@@ -113,9 +113,6 @@ static struct exclude *exclude;
 /* Grand total size of all args, in bytes. */
 static uintmax_t tot_size = 0;
 
-/* Nonzero indicates that du should exit with EXIT_FAILURE upon completion.  */
-static int G_fail;
-
 #define IS_DIR_TYPE(Type)      \
   ((Type) == FTS_DP            \
    || (Type) == FTS_DNR)
@@ -247,10 +244,9 @@ entry_compare (void const *x, void const
 }
 
 /* Try to insert the INO/DEV pair into the global table, HTAB.
-   If the pair is successfully inserted, return zero.
-   Upon failed memory allocation exit nonzero.
-   If the pair is already in the table, return nonzero.  */
-static int
+   Return true if the pair is successfully inserted,
+   false if the pair is already in the table.  */
+static bool
 hash_ins (ino_t ino, dev_t dev)
 {
   struct entry *ent;
@@ -270,13 +266,13 @@ hash_ins (ino_t ino, dev_t dev)
   if (ent_from_table == ent)
     {
       /* Insertion succeeded.  */
-      return 0;
+      return true;
     }
 
   /* That pair is already in the table, so ENT was not inserted.  Free it.  */
   free (ent);
 
-  return 1;
+  return false;
 }
 
 /* Initialize the hash table.  */
@@ -313,14 +309,15 @@ print_size (uintmax_t n_bytes, const cha
 /* This function is called once for every file system object that fts
    encounters.  fts does a depth-first traversal.  This function knows
    that and accumulates per-directory totals based on changes in
-   the depth of the current entry.  */
+   the depth of the current entry.  It returns true on success.  */
 
-static void
+static bool
 process_file (FTS *fts, FTSENT *ent)
 {
+  bool ok;
   uintmax_t size;
   uintmax_t size_to_print;
-  static int first_call = 1;
+  size_t level;
   static size_t prev_level;
   static size_t n_alloc;
   /* The sum of the st_size values of all entries in the single directory
@@ -332,11 +329,11 @@ process_file (FTS *fts, FTSENT *ent)
   /* The sum of the sizes of all entries in the hierarchy at or below the
      directory at the specified level.  */
   static uintmax_t *sum_subdir;
-  int print = 1;
+  bool print = true;
 
   const char *file = ent->fts_path;
   const struct stat *sb = ent->fts_statp;
-  int skip;
+  bool skip;
 
   /* If necessary, set FTS_SKIP before returning.  */
   skip = excluded_filename (exclude, ent->fts_name);
@@ -347,23 +344,22 @@ process_file (FTS *fts, FTSENT *ent)
     {
     case FTS_NS:
       error (0, ent->fts_errno, _("cannot access %s"), quote (file));
-      G_fail = 1;
-      return;
+      return false;
 
     case FTS_ERR:
       /* if (S_ISDIR (ent->fts_statp->st_mode) && FIXME */
       error (0, ent->fts_errno, _("%s"), quote (file));
-      G_fail = 1;
-      return;
+      return false;
 
     case FTS_DNR:
       /* Don't return just yet, since although the directory is not readable,
         we were able to stat it, so we do have a size.  */
       error (0, ent->fts_errno, _("cannot read directory %s"), quote (file));
-      G_fail = 1;
+      ok = false;
       break;
 
     default:
+      ok = true;
       break;
     }
 
@@ -371,7 +367,7 @@ process_file (FTS *fts, FTSENT *ent)
      or if it's the second encounter for a skipped directory, then
      return right away.  */
   if (ent->fts_info == FTS_D || skip)
-    return;
+    return ok;
 
   /* If the file is being excluded or if it has already been counted
      via a hard link, then don't let it contribute to the sums.  */
@@ -379,13 +375,13 @@ process_file (FTS *fts, FTSENT *ent)
       || (!opt_count_all
          && ! S_ISDIR (sb->st_mode)
          && 1 < sb->st_nlink
-         && hash_ins (sb->st_ino, sb->st_dev)))
+         && ! hash_ins (sb->st_ino, sb->st_dev)))
     {
       /* Note that we must not simply return here.
         We still have to update prev_level and maybe propagate
         some sums up the hierarchy.  */
       size = 0;
-      print = 0;
+      print = false;
     }
   else
     {
@@ -394,49 +390,44 @@ process_file (FTS *fts, FTSENT *ent)
              : ST_NBLOCKS (*sb) * ST_NBLOCKSIZE);
     }
 
-  if (first_call)
+  level = ent->fts_level;
+  size_to_print = size;
+
+  if (n_alloc == 0)
     {
-      n_alloc = ent->fts_level + 10;
-      sum_ent = XCALLOC (uintmax_t, n_alloc);
-      sum_subdir = XCALLOC (uintmax_t, n_alloc);
+      n_alloc = level + 10;
+      sum_ent = xcalloc (n_alloc, sizeof *sum_ent);
+      sum_subdir = xcalloc (n_alloc, sizeof *sum_subdir);
     }
   else
     {
-      /* FIXME: it's a shame that we need these `size_t' casts to avoid
-        warnings from gcc about `comparison between signed and unsigned'.
-        Probably unavoidable, assuming that the struct members
-        are of type `int' (historical), since I want variables like
-        n_alloc and prev_level to have types that make sense.  */
-      if (n_alloc <= (size_t) ent->fts_level)
-       {
-         n_alloc = ent->fts_level * 2;
-         sum_ent = XREALLOC (sum_ent, uintmax_t, n_alloc);
-         sum_subdir = XREALLOC (sum_subdir, uintmax_t, n_alloc);
-       }
-    }
-
-  size_to_print = size;
-
-  if (! first_call)
-    {
-      if ((size_t) ent->fts_level == prev_level)
+      if (level == prev_level)
        {
          /* This is usually the most common case.  Do nothing.  */
        }
-      else if (ent->fts_level > prev_level)
+      else if (level > prev_level)
        {
          /* Descending the hierarchy.
             Clear the accumulators for *all* levels between prev_level
             and the current one.  The depth may change dramatically,
             e.g., from 1 to 10.  */
-         int i;
-         for (i = prev_level + 1; i <= ent->fts_level; i++)
+         size_t i;
+
+         if (n_alloc <= level)
+           {
+             sum_ent = xnrealloc (sum_ent, level, 2 * sizeof *sum_ent);
+             sum_subdir = xnrealloc (sum_subdir, level,
+                                     2 * sizeof *sum_subdir);
+             n_alloc = level * 2;
+           }
+
+         for (i = prev_level + 1; i <= level; i++)
            {
              sum_ent[i] = 0;
              sum_subdir[i] = 0;
            }
        }
-      else /* ent->fts_level < prev_level */
+      else /* level < prev_level */
        {
          /* Ascending the hierarchy.
             Process a directory only after all entries in that
@@ -444,22 +435,20 @@ process_file (FTS *fts, FTSENT *ent)
             propagate sums from the children (prev_level) to the parent.
             Here, the current level is always one smaller than the
             previous one.  */
-         assert ((size_t) ent->fts_level == prev_level - 1);
+         assert (level == prev_level - 1);
          size_to_print += sum_ent[prev_level];
          if (!opt_separate_dirs)
            size_to_print += sum_subdir[prev_level];
-         sum_subdir[ent->fts_level] += (sum_ent[prev_level]
-                                        + sum_subdir[prev_level]);
+         sum_subdir[level] += sum_ent[prev_level] + sum_subdir[prev_level];
        }
     }
 
-  prev_level = ent->fts_level;
-  first_call = 0;
+  prev_level = level;
 
   /* Let the size of a directory entry contribute to the total for the
      containing directory, unless --separate-dirs (-S) is specified.  */
   if ( ! (opt_separate_dirs && IS_DIR_TYPE (ent->fts_info)))
-    sum_ent[ent->fts_level] += size;
+    sum_ent[level] += size;
 
   /* Even if this directory is unreadable or we can't chdir into it,
      do let its size contribute to the total, ... */
@@ -468,16 +457,16 @@ process_file (FTS *fts, FTSENT *ent)
   /* ... but don't print out a total for it, since without the size(s)
      of any potential entries, it could be very misleading.  */
   if (ent->fts_info == FTS_DNR)
-    return;
+    return ok;
 
   /* If we're not counting an entry, e.g., because it's a hard link
      to a file we've already counted (and --count-links), then don't
      print a line for it.  */
   if (!print)
-    return;
+    return ok;
 
-  if ((IS_DIR_TYPE (ent->fts_info) && ent->fts_level <= max_depth)
-      || ((opt_all && ent->fts_level <= max_depth) || ent->fts_level == 0))
+  if ((IS_DIR_TYPE (ent->fts_info) && level <= max_depth)
+      || ((opt_all && level <= max_depth) || level == 0))
     {
       print_only_size (size_to_print);
       fputc ('\t', stdout);
@@ -485,18 +474,19 @@ process_file (FTS *fts, FTSENT *ent)
       fputc (opt_nul_terminate_output ? '\0' : '\n', stdout);
       fflush (stdout);
     }
+
+  return ok;
 }
 
 /* Recursively print the sizes of the directories (and, if selected, files)
    named in FILES, the last entry of which is NULL.
    BIT_FLAGS controls how fts works.
-   If the fts_open call fails, exit nonzero.
-   Otherwise, return nonzero upon error.  */
+   Return true if successful.  */
 
 static bool
 du_files (char **files, int bit_flags)
 {
-  bool fail = false;
+  bool ok = true;
 
   if (*files)
     {
@@ -513,13 +503,13 @@ du_files (char **files, int bit_flags)
                {
                  /* FIXME: try to give a better message  */
                  error (0, errno, _("fts_read failed"));
-                 fail = true;
+                 ok = false;
                }
              break;
            }
          FTS_CROSS_CHECK (fts);
 
-         process_file (fts, ent);
+         ok &= process_file (fts, ent);
        }
 
       /* Ignore failure, since the only way it can do so is in failing to
@@ -531,7 +521,7 @@ du_files (char **files, int bit_flags)
   if (print_grand_total)
     print_size (tot_size, _("total"));
 
-  return fail;
+  return ok;
 }
 
 int
@@ -539,17 +529,17 @@ main (int argc, char **argv)
 {
   int c;
   char *cwd_only[2];
-  int max_depth_specified = 0;
+  bool max_depth_specified = false;
   char **files;
-  bool fail;
+  bool ok = true;
   char *files_from = NULL;
   struct Tokens tok;
 
   /* Bit flags that control how fts works.  */
   int bit_flags = FTS_PHYSICAL | FTS_TIGHT_CYCLE_CHECK;
 
-  /* If nonzero, display only a total for each argument. */
-  int opt_summarize_only = 0;
+  /* If true, display only a total for each argument. */
+  bool opt_summarize_only = false;
 
   cwd_only[0] = ".";
   cwd_only[1] = NULL;
@@ -567,11 +557,9 @@ main (int argc, char **argv)
   human_output_opts = human_options (getenv ("DU_BLOCK_SIZE"), false,
                                     &output_block_size);
 
-  fail = false;
   while ((c = getopt_long (argc, argv, DEBUG_OPT "0abchHklmsxB:DLPSX:",
                           long_options, NULL)) != -1)
     {
-      long int tmp_long;
       switch (c)
        {
        case 0:                 /* Long option. */
@@ -579,7 +567,7 @@ main (int argc, char **argv)
 
 #if DU_DEBUG
        case 'd':
-         fts_debug = 1;
+         fts_debug = true;
          break;
 #endif
 
@@ -588,21 +576,21 @@ main (int argc, char **argv)
          break;
 
        case 'a':
-         opt_all = 1;
+         opt_all = true;
          break;
 
        case APPARENT_SIZE_OPTION:
-         apparent_size = 1;
+         apparent_size = true;
          break;
 
        case 'b':
-         apparent_size = 1;
+         apparent_size = true;
          human_output_opts = 0;
          output_block_size = 1;
          break;
 
        case 'c':
-         print_grand_total = 1;
+         print_grand_total = true;
          break;
 
        case 'h':
@@ -625,18 +613,21 @@ main (int argc, char **argv)
          break;
 
        case MAX_DEPTH_OPTION:          /* --max-depth=N */
-         if (xstrtol (optarg, NULL, 0, &tmp_long, NULL) == LONGINT_OK
-             && 0 <= tmp_long && tmp_long <= INT_MAX)
-           {
-             max_depth_specified = 1;
-             max_depth = (int) tmp_long;
-           }
-         else
-           {
-             error (0, 0, _("invalid maximum depth %s"),
-                    quote (optarg));
-             fail = true;
-           }
+         {
+           unsigned long int tmp_ulong;
+           if (xstrtoul (optarg, NULL, 0, &tmp_ulong, NULL) == LONGINT_OK
+               && tmp_ulong <= SIZE_MAX)
+             {
+               max_depth_specified = true;
+               max_depth = tmp_ulong;
+             }
+           else
+             {
+               error (0, 0, _("invalid maximum depth %s"),
+                      quote (optarg));
+               ok = false;
+             }
+         }
          break;
 
        case 'm': /* obsolescent: FIXME: remove in 2005. */
@@ -645,11 +636,11 @@ main (int argc, char **argv)
          break;
 
        case 'l':
-         opt_count_all = 1;
+         opt_count_all = true;
          break;
 
        case 's':
-         opt_summarize_only = 1;
+         opt_summarize_only = true;
          break;
 
        case 'x':
@@ -673,7 +664,7 @@ main (int argc, char **argv)
          break;
 
        case 'S':
-         opt_separate_dirs = 1;
+         opt_separate_dirs = true;
          break;
 
        case 'X':
@@ -681,7 +672,7 @@ main (int argc, char **argv)
                                EXCLUDE_WILDCARDS, '\n'))
            {
              error (0, errno, "%s", quotearg_colon (optarg));
-             fail = true;
+             ok = false;
            }
          break;
 
@@ -698,14 +689,14 @@ main (int argc, char **argv)
        case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
 
        default:
-         fail = true;
+         ok = false;
        }
     }
 
-  if (fail)
+  if (!ok)
     usage (EXIT_FAILURE);
 
-  if (opt_all && opt_summarize_only)
+  if (opt_all & opt_summarize_only)
     {
       error (0, 0, _("cannot both summarize and show all entries"));
       usage (EXIT_FAILURE);
@@ -719,9 +710,8 @@ main (int argc, char **argv)
 
   if (opt_summarize_only && max_depth_specified && max_depth != 0)
     {
-      error (0, 0,
-            _("warning: summarizing conflicts with --max-depth=%d"),
-              max_depth);
+      unsigned long int d = max_depth;
+      error (0, 0, _("warning: summarizing conflicts with --max-depth=%lu"), 
d);
       usage (EXIT_FAILURE);
     }
 
@@ -797,10 +787,10 @@ main (int argc, char **argv)
          }
       }
 
-    fail = (i != j);
+    ok = (i == j);
   }
 
-  fail |= du_files (files, bit_flags);
+  ok &= du_files (files, bit_flags);
 
   /* This isn't really necessary, but it does ensure we
      exercise this function.  */
@@ -809,5 +799,5 @@ main (int argc, char **argv)
 
   hash_free (htab);
 
-  exit (fail || G_fail ? EXIT_FAILURE : EXIT_SUCCESS);
+  exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 }




reply via email to

[Prev in Thread] Current Thread [Next in Thread]