Re: [Bug-tar] Error "Cannot allocate memory" incorrectly reported in some cases.

[Paul Eggert]

"Dmitry V. Levin" <address@hidden> writes:

> if errno is used to test for readdir() errors like in this example,
> then errno should be cleared before each readdir() call, not just
> before first one.

Yes, and the patch I installed yesterday to savedir.c does just that.

> Similar issue exists in lib/getcwd.c

Thanks for mentioning this.  I installed this patch to fix it, in both
coreutils and gnulib.

2005-10-29  Paul Eggert  <address@hidden>

        * lib/getcwd.c (__getcwd): Don't assume that system calls after readdir
        leave errno alone.  Problem reported by Dmitry V. Levin.

--- lib/getcwd.c        27 Sep 2005 08:33:47 -0000      1.16
+++ lib/getcwd.c        30 Oct 2005 01:29:05 -0000
@@ -201,6 +201,8 @@ __getcwd (char *buf, size_t size)
       ino_t dotino;
       bool mount_point;
       int parent_status;
+      size_t dirroom;
+      size_t namlen;
 
       /* Look at the parent directory.  */
 #ifdef AT_FDCWD
@@ -241,11 +243,20 @@ __getcwd (char *buf, size_t size)
        goto lose;
       dotlist[dotlen++] = '/';
 #endif
-      /* Clear errno to distinguish EOF from error if readdir returns
-        NULL.  */
-      __set_errno (0);
-      while ((d = __readdir (dirstream)) != NULL)
+      for (;;)
        {
+         /* Clear errno to distinguish EOF from error if readdir returns
+            NULL.  */
+         __set_errno (0);
+         d = __readdir (dirstream);
+         if (d == NULL)
+           {
+             if (errno == 0)
+               /* EOF on dirstream, which means that the current directory
+                  has been removed.  */
+               __set_errno (ENOENT);
+             goto lose;
+           }
          if (d->d_name[0] == '.' &&
              (d->d_name[1] == '\0' ||
               (d->d_name[1] == '.' && d->d_name[2] == '\0')))
@@ -303,48 +314,38 @@ __getcwd (char *buf, size_t size)
                break;
            }
        }
-      if (d == NULL)
-       {
-         if (errno == 0)
-           /* EOF on dirstream, which means that the current directory
-              has been removed.  */
-           __set_errno (ENOENT);
-         goto lose;
-       }
-      else
-       {
-         size_t dirroom = dirp - dir;
-         size_t namlen = _D_EXACT_NAMLEN (d);
 
-         if (dirroom <= namlen)
+      dirroom = dirp - dir;
+      namlen = _D_EXACT_NAMLEN (d);
+
+      if (dirroom <= namlen)
+       {
+         if (size != 0)
            {
-             if (size != 0)
-               {
-                 __set_errno (ERANGE);
-                 goto lose;
-               }
-             else
-               {
-                 char *tmp;
-                 size_t oldsize = allocated;
+             __set_errno (ERANGE);
+             goto lose;
+           }
+         else
+           {
+             char *tmp;
+             size_t oldsize = allocated;
 
-                 allocated += MAX (allocated, namlen);
-                 if (allocated < oldsize
-                     || ! (tmp = realloc (dir, allocated)))
-                   goto memory_exhausted;
+             allocated += MAX (allocated, namlen);
+             if (allocated < oldsize
+                 || ! (tmp = realloc (dir, allocated)))
+               goto memory_exhausted;
 
-                 /* Move current contents up to the end of the buffer.
-                    This is guaranteed to be non-overlapping.  */
-                 dirp = memcpy (tmp + allocated - (oldsize - dirroom),
-                                tmp + dirroom,
-                                oldsize - dirroom);
-                 dir = tmp;
-               }
+             /* Move current contents up to the end of the buffer.
+                This is guaranteed to be non-overlapping.  */
+             dirp = memcpy (tmp + allocated - (oldsize - dirroom),
+                            tmp + dirroom,
+                            oldsize - dirroom);
+             dir = tmp;
            }
-         dirp -= namlen;
-         memcpy (dirp, d->d_name, namlen);
-         *--dirp = '/';
        }
+      dirp -= namlen;
+      memcpy (dirp, d->d_name, namlen);
+      *--dirp = '/';
 
       thisdev = dotdev;
       thisino = dotino;