Re: [PATCH] src/remove.c: cache_fstatat() broken on systems with negative errnos

[Jim Meyering]

Ingo Weinhold <address@hidden> wrote:
> on systems with negative errno values (BeOS, Haiku) the method used in the
> cache_fstatat() function in src/remove.c is broken (st_size will become
> positive on error). The attached patch changes the function to use the
> st_ino field to store the errno value instead, which should work on all
> platforms.

Thanks for the patch.
I've applied it and updated NEWS and c99-to-c89.diff:
--------------------------------------------

>From e73dfc3899c2a622c91c0948610a1c5c1e1972d1 Mon Sep 17 00:00:00 2001
From: Ingo Weinhold <address@hidden>
Date: Fri, 21 Mar 2008 14:10:27 +0100
Subject: [PATCH] remove.c: Accommodate systems with negative errno values.

* src/remove.c (cache_fstatat): Store errno value directly in
the st_ino field, rather than trying to shoehorn it into st_size.
This is required at least on BeOS and Haiku.

Signed-off-by: Jim Meyering <address@hidden>
---
 src/remove.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/remove.c b/src/remove.c
index 1658fb9..9c6dc9e 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -171,16 +171,19 @@ static size_t g_n_allocated;

 /* Like fstatat, but cache the result.  If ST->st_size is -1, the
    status has not been gotten yet.  If less than -1, fstatat failed
-   with errno == -1 - ST->st_size.  Otherwise, the status has already
+   with errno == ST->st_ino.  Otherwise, the status has already
    been gotten, so return 0.  */
 static int
 cache_fstatat (int fd, char const *file, struct stat *st, int flag)
 {
   if (st->st_size == -1 && fstatat (fd, file, st, flag) != 0)
-    st->st_size = -1 - errno;
+    {
+      st->st_size = -2;
+      st->st_ino = errno;
+    }
   if (0 <= st->st_size)
     return 0;
-  errno = -1 - st->st_size;
+  errno = (int) st->st_ino;
   return -1;
 }

--
1.5.5.rc0.22.g467c


>From 6e5bbc267a65946bc5f1cfc69301c6c378c8795a Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 21 Mar 2008 14:15:54 +0100
Subject: [PATCH] Write NEWS and update c99-to-c89 patch for today's rm 
improvement.

* NEWS: call this a "portability improvement" ;-)
* src/c99-to-c89.diff: Adjust remove.c offsets.

Signed-off-by: Jim Meyering <address@hidden>
---
 NEWS                |    5 +++++
 src/c99-to-c89.diff |   50 +++++++++++++++++++++++++-------------------------
 2 files changed, 30 insertions(+), 25 deletions(-)

diff --git a/NEWS b/NEWS
index 93f1331..3433668 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,11 @@ GNU coreutils NEWS                                    -*- 
outline -*-

   seq gives better diagnostics for invalid formats.

+** Portability
+
+  rm now works properly even on systems like BeOS and Haiku,
+  which have negative errno values.
+
 ** Consistency

   install, mkdir, rmdir and split now write --verbose output to stdout,
diff --git a/src/c99-to-c89.diff b/src/c99-to-c89.diff
index 286f276..98c93c6 100644
--- a/src/c99-to-c89.diff
+++ b/src/c99-to-c89.diff
@@ -1,7 +1,25 @@
+--- src/id.c   2008-03-04 18:50:55.000000000 +0100
++++ src/id.c   2008-03-04 19:32:24.000000000 +0100
+@@ -196,6 +196,7 @@ of a different user"));
+     error (EXIT_FAILURE, 0,
+          _("cannot print only names or real IDs in default format"));
+
++  {
+   char const *user_name;
+   if (argc - optind == 1)
+     {
+@@ -239,6 +240,7 @@ of a different user"));
+     {
+       print_full_info (user_name);
+     }
++  }
+   putchar ('\n');
+
+   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
 diff -upr src/remove.c src/remove.c
 --- src/remove.c       2007-07-23 12:56:20.000000000 +0200
 +++ src/remove.c       2007-07-23 13:03:12.000000000 +0200
-@@ -261,9 +261,10 @@ pop_dir (Dirstack_state *ds)
+@@ -264,9 +264,10 @@ pop_dir (Dirstack_state *ds)
  {
    size_t n_lengths = obstack_object_size (&ds->len_stack) / sizeof (size_t);
    size_t *length = obstack_base (&ds->len_stack);
@@ -13,7 +31,7 @@ diff -upr src/remove.c src/remove.c
    assert (top_len >= 2);

    /* Pop the specified length of file name.  */
-@@ -421,10 +422,11 @@ AD_stack_top (Dirstack_state const *ds)
+@@ -424,10 +425,11 @@ AD_stack_top (Dirstack_state const *ds)
  static void
  AD_stack_pop (Dirstack_state *ds)
  {
@@ -26,7 +44,7 @@ diff -upr src/remove.c src/remove.c
    if (top->unremovable)
      hash_free (top->unremovable);
    obstack_blank (&ds->Active_dir, -(int) sizeof (struct AD_ent));
-@@ -876,6 +878,7 @@ prompt (int fd_cwd, Dirstack_state const
+@@ -879,6 +881,7 @@ prompt (int fd_cwd, Dirstack_state const
            break;
          }

@@ -34,7 +52,7 @@ diff -upr src/remove.c src/remove.c
        char const *quoted_name = quote (full_filename (filename));

        if (0 < write_protected)
-@@ -915,6 +918,7 @@ prompt (int fd_cwd, Dirstack_state const
+@@ -918,6 +921,7 @@ prompt (int fd_cwd, Dirstack_state const
                    : _("%s: remove %s %s? ")),
                   program_name, file_type (sbuf), quoted_name);
        }
@@ -42,7 +60,7 @@ diff -upr src/remove.c src/remove.c

        if (!yesno ())
        return RM_USER_DECLINED;
-@@ -1534,6 +1538,7 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1537,6 +1541,7 @@ rm_1 (Dirstack_state *ds, char const *fi
        return RM_ERROR;
      }

@@ -50,7 +68,7 @@ diff -upr src/remove.c src/remove.c
    struct stat st;
    cache_stat_init (&st);
    cycle_check_init (&ds->cycle_check_state);
-@@ -1556,6 +1561,7 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1559,6 +1564,7 @@ rm_1 (Dirstack_state *ds, char const *fi
    AD_push_initial (ds);
    AD_INIT_OTHER_MEMBERS ();

@@ -58,7 +76,7 @@ diff -upr src/remove.c src/remove.c
    enum RM_status status = remove_entry (AT_FDCWD, ds, filename,
                                        DT_UNKNOWN, &st, x);
    if (status == RM_NONEMPTY_DIR)
-@@ -1574,6 +1580,8 @@ rm_1 (Dirstack_state *ds, char const *fi
+@@ -1577,6 +1583,8 @@ rm_1 (Dirstack_state *ds, char const *fi
    ds_clear (ds);
    return status;
  }
@@ -152,21 +170,3 @@ diff -upr src/shred.c src/shred.c
                  if (errnum == EIO && 0 <= size && (soff | SECTOR_MASK) < lim)
                    {
                      size_t soff1 = (soff | SECTOR_MASK) + 1;
---- src/id.c   2008-03-04 18:50:55.000000000 +0100
-+++ src/id.c   2008-03-04 19:32:24.000000000 +0100
-@@ -196,6 +196,7 @@ of a different user"));
-     error (EXIT_FAILURE, 0,
-          _("cannot print only names or real IDs in default format"));
-
-+  {
-   char const *user_name;
-   if (argc - optind == 1)
-     {
-@@ -239,6 +240,7 @@ of a different user"));
-     {
-       print_full_info (user_name);
-     }
-+  }
-   putchar ('\n');
-
-   exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
--
1.5.5.rc0.22.g467c