"Axel Dörfler" <[EMAIL PROTECTED]> wrote:
> the enclosed patch changes the following (remove.c against HEAD):
...
Thank you for the patch.
Sorry not to have replied sooner.
At first I hesitated to make a relatively invasive change
to remove.c solely to accommodate a fringe target like BeOS/Haiku.
Then I noticed the change wasn't quite right, and finally I
discovered a real bug in rm (just fixed, separately).
> diff --git a/src/remove.c b/src/remove.c
...
So far, so good.
> @@ -839,15 +842,18 @@ prompt (int fd_cwd, Dirstack_state const *ds, char
> const *filename,
> if (x->interactive == RMI_NEVER)
> return RM_OK;
>
> + errno = 0;
> +
> if (!x->ignore_missing_files
> && ((x->interactive == RMI_ALWAYS) || x->stdin_tty)
> && dirent_type != DT_LNK)
> write_protected = write_protected_non_symlink (fd_cwd, filename, ds,
> sbuf);
>
> - if (write_protected || x->interactive == RMI_ALWAYS)
> + if (write_protected || errno || x->interactive == RMI_ALWAYS)
> {
> - if (write_protected <= 0 && dirent_type == DT_UNKNOWN)
> + if (errno == 0 && dirent_type == DT_UNKNOWN)
> {
> + /* Might fail, e.g., for `rm '''. */
> if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) == 0)
> {
> if (S_ISLNK (sbuf->st_mode))
> @@ -857,14 +863,9 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const
> *filename,
> /* Otherwise it doesn't matter, so leave it DT_UNKNOWN. */
> *pdirent_type = dirent_type;
> }
> - else
> - {
> - /* This happens, e.g., with `rm '''. */
> - write_protected = errno;
> - }
> }
>
> - if (write_protected <= 0)
> + if (errno == 0)
However, at this point we can no longer rely on errno to have
retained its value from the call to write_protected_non_symlink.
It may have been changed via the cache_fstatat call.
So I reworked things so that the errno value is saved
in its own variable, and changed several conditions to
make it clearer that the main artifact of this change is
the return-value change of sign.
Here's the patch I expect to push:
>From: Jim Meyering <[EMAIL PROTECTED]>
Date: Mon, 24 Mar 2008 17:38:27 +0100
Subject: [PATCH] remove.c: accommodate systems with negative errno values
This is required at least on Haiku and BeOS.
* src/remove.c (write_protected_non_symlink): Return 1 for a write-
protected non-symlink, 0 if we determine it's not, and -1 upon
error (setting errno accordingly only in this final case).
(prompt): Deal with the changed semantics of the above function.
Based on this patch from Axel Dörfler:
http://thread.gmane.org/gmane.comp.gnu.coreutils.bugs/13071
Signed-off-by: Jim Meyering <[EMAIL PROTECTED]>
---
THANKS | 1 +
src/remove.c | 38 +++++++++++++++++++++++++-------------
2 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/THANKS b/THANKS
index d38e51f..d746b29 100644
--- a/THANKS
+++ b/THANKS
@@ -21,6 +21,7 @@ Albert Hopkins [EMAIL PROTECTED]
Alberto Accomazzi [EMAIL PROTECTED]
aldomel [EMAIL PROTECTED]
Alen Muzinic [EMAIL PROTECTED]
+Axel Dörfler [EMAIL PROTECTED]
Alexandre Duret-Lutz [EMAIL PROTECTED]
Alexey Solovyov [EMAIL PROTECTED]
Alexey Vyskubov [EMAIL PROTECTED]
diff --git a/src/remove.c b/src/remove.c
index 7ba1e38..07c2f71 100644
--- a/src/remove.c
+++ b/src/remove.c
@@ -728,9 +728,9 @@ AD_is_removable (Dirstack_state const *ds, char const *file)
return ! (top->unremovable && hash_lookup (top->unremovable, file));
}
-/* Return -1 if FILE is an unwritable non-symlink,
+/* Return 1 if FILE is an unwritable non-symlink,
0 if it is writable or some other type of file,
- a positive error number if there is some problem in determining the answer.
+ -1 and set errno if there is some problem in determining the answer.
Set *BUF to the file status.
This is to avoid calling euidaccess when FILE is a symlink. */
static int
@@ -742,7 +742,7 @@ write_protected_non_symlink (int fd_cwd,
if (can_write_any_file ())
return 0;
if (cache_fstatat (fd_cwd, file, buf, AT_SYMLINK_NOFOLLOW) != 0)
- return errno;
+ return -1;
if (S_ISLNK (buf->st_mode))
return 0;
/* Here, we know FILE is not a symbolic link. */
@@ -799,15 +799,18 @@ write_protected_non_symlink (int fd_cwd,
= obstack_object_size (&ds->dir_stack) + strlen (file);
if (MIN (PATH_MAX, 8192) <= file_name_len)
- return euidaccess_stat (buf, W_OK) ? 0 : -1;
+ return ! euidaccess_stat (buf, W_OK);
if (euidaccess (xfull_filename (ds, file), W_OK) == 0)
return 0;
if (errno == EACCES)
- return -1;
+ {
+ errno = 0;
+ return 1;
+ }
/* Perhaps some other process has removed the file, or perhaps this
is a buggy NFS client. */
- return errno;
+ return -1;
}
}
@@ -839,14 +842,19 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const
*filename,
if (x->interactive == RMI_NEVER)
return RM_OK;
+ int wp_errno = 0;
+
if (!x->ignore_missing_files
&& ((x->interactive == RMI_ALWAYS) || x->stdin_tty)
&& dirent_type != DT_LNK)
- write_protected = write_protected_non_symlink (fd_cwd, filename, ds, sbuf);
+ {
+ write_protected = write_protected_non_symlink (fd_cwd, filename, ds,
sbuf);
+ wp_errno = errno;
+ }
if (write_protected || x->interactive == RMI_ALWAYS)
{
- if (write_protected <= 0 && dirent_type == DT_UNKNOWN)
+ if (0 <= write_protected && dirent_type == DT_UNKNOWN)
{
if (cache_fstatat (fd_cwd, filename, sbuf, AT_SYMLINK_NOFOLLOW) == 0)
{
@@ -860,11 +868,12 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const
*filename,
else
{
/* This happens, e.g., with `rm '''. */
- write_protected = errno;
+ write_protected = -1;
+ wp_errno = errno;
}
}
- if (write_protected <= 0)
+ if (0 <= write_protected)
switch (dirent_type)
{
case DT_LNK:
@@ -875,15 +884,18 @@ prompt (int fd_cwd, Dirstack_state const *ds, char const
*filename,
case DT_DIR:
if (!x->recursive)
- write_protected = EISDIR;
+ {
+ write_protected = -1;
+ wp_errno = EISDIR;
+ }
break;
}
char const *quoted_name = quote (full_filename (filename));
- if (0 < write_protected)
+ if (write_protected < 0)
{
- error (0, write_protected, _("cannot remove %s"), quoted_name);
+ error (0, wp_errno, _("cannot remove %s"), quoted_name);
return RM_ERROR;
}
--
1.5.5.rc1.13.g79388
_______________________________________________
Bug-coreutils mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/bug-coreutils
