bug#11100: Racy code in copy.c

[Jim Meyering]

On Thu, Nov 18, 2021 at 8:38 AM Paul Eggert  wrote:
> I spotted a SELinux security-context race introduced by the circa-2012
> fix for Bug#11100, and installed the attached patch into coreutils
> master. This also gets rid of a label and goto (which is what led me to
> find the issue).

Nice! Thanks for finding and fixing my old bug.

bug#50714: OS X, one failure: tests/tail-2/pipe-f.sh

[Jim Meyering]

Uname -v reports this:
Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021;
root:xnu-7195.141.6~3/RELEASE_X86_64

Sorry, I don't have time to delve into this, but here's the log from
the sole test failure:


pipe-f.log
Description: Binary data

bug#50611: one-byte (write) heap-buffer-underrun

[Jim Meyering]

Thanks for all your recent changes! I built+tested with ASAN on Fedora 34:

Configure and build as usual, then "make clean" and do this:
> san='-fsanitize-address-use-after-scope -fsanitize=address -static-libasan'; 
> ASAN_OPTIONS=detect_leaks=0 , CFLAGS='-O -ggdb3' AM_CFLAGS="$san" 
> AM_LDFLAGS="$san" check

(but that first -f option may be obsolete, because it seems to provoke
spurious failure of the stdbuf test and help-version tests)

That exposed this (and similar in an md5sum tests):

md5sum: test ck-strict-1: stderr mismatch, comparing ck-strict-1.2
(expected) and ck-strict-1.E (actual)
*** ck-strict-1.2   Wed Sep 15 17:16:39 2021
--- ck-strict-1.E   Wed Sep 15 17:16:39 2021
***
*** 1 
! md5sum: WARNING: 1 line is improperly formatted
--- 1,47 
! =
! ==1752792==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x60c0003f at pc 0x004d7387 bp 0x7fff29bac390 sp
0x7fff29bac388
! READ of size 1 at 0x60c0003f thread T0
! #0 0x4d7386 in digest_check src/digest.c:1076
! #1 0x4d7386 in main src/digest.c:1492
! #2 0x7ff1f089db74 in __libc_start_main (/lib64/libc.so.6+0x27b74)
! #3 0x40754d in _start (/home/j/w/co/cu/src/md5sum+0x40754d)
!
! 0x60c0003f is located 1 bytes to the left of 120-byte region
[0x60c00040,0x60c000b8)
! allocated by thread T0 here:
! #0 0x492417 in __interceptor_malloc
/home/j/w/co/gcc/libsanitizer/asan/asan_malloc_linux.cpp:129
! #1 0x7ff1f08ec903 in _IO_getdelim (/lib64/libc.so.6+0x76903)
! #2 0x49208f  (/home/j/w/co/cu/src/md5sum+0x49208f)
!
! SUMMARY: AddressSanitizer: heap-buffer-overflow src/digest.c:1076 in
digest_check
! Shadow bytes around the buggy address:
!   0x0c187fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
!   0x0c187fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
!   0x0c187fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
!   0x0c187fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
!   0x0c187fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
! =>0x0c187fff8000: fa fa fa fa fa fa fa[fa]00 00 00 00 00 00 00 00
!   0x0c187fff8010: 00 00 00 00 00 00 00 fa fa fa fa fa fa fa fa fa
!   0x0c187fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
!   0x0c187fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
!   0x0c187fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
!   0x0c187fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
! Shadow byte legend (one shadow byte represents 8 application bytes):
!   Addressable:   00
!   Partially addressable: 01 02 03 04 05 06 07
!   Heap left redzone:   fa
!   Freed heap region:   fd
!   Stack left redzone:  f1
!   Stack mid redzone:   f2
!   Stack right redzone: f3
!   Stack after return:  f5
!   Stack use after scope:   f8
!   Global redzone:  f9
!   Global init order:   f6
!   Poisoned by user:f7
!   Container overflow:  fc
!   Array cookie:ac
!   Intra object redzone:bb
!   ASan internal:   fe
!   Left alloca redzone: ca
!   Right alloca redzone:cb
! ==1752792==ABORTING

bug#50167: fixes for "fmt - -" etc.

[Jim Meyering]

On Wed, Aug 25, 2021, 4:37 PM Bernhard Voelker 
wrote:

> On 8/25/21 10:38 AM, Jim Meyering wrote:
> > * cfg.mk (exclude_file_name_regexp--sc_system_h_headers):
> > Add find-mount-point.h to the regexp.
>
> +1
> even better, thanks.
>

Thanks. Pushed.

>

bug#50167: fixes for "fmt - -" etc.

[Jim Meyering]

On Tue, Aug 24, 2021 at 11:10 PM Paul Eggert  wrote:
> On 8/24/21 12:42 PM, Bernhard Voelker wrote:
> > Was there a particular reason to include stdlib.h?
>
> It's needed to declare 'free', which is used by  _GL_ATTRIBUTE_DEALLOC_FREE.
>
> I added "#include " so that find-mount-point.h would be an
> independent header, potentially useful outside coreutils. If that's the
> intent, we should fix "make syntax-check" to not complain about this
> situation. If it's not the intent then your patch looks good.
>
> Jim, what's your opinion on this?

I propose this change:


0001-maint-avoid-new-syntax-check-failure.patch
Description: Binary data

bug#49741: basenc --base64url decoding bug

[Jim Meyering]

On Tue, Aug 17, 2021 at 2:02 AM Pádraig Brady  wrote:
> On 16/08/2021 22:17, Assaf Gordon wrote:
> > Hello Emil and all,
> >
> > Thanks for the clear and easily reproducible bug report.
> >
> > Attached a suggested fix.
> > Comments very welcomed,
>
> minor nit in NEWS:
> s/silently discard/silently discards/
>
> Otherwise it looks good!

Nice, indeed!
a nit in the commit log:

-The input buffer was not divisible by 3
+The input buffer length was not divisible by 3

bug#45093: Character 149 causing ASCII BEL output to console in Windoze port of Gnu CoreUtils

[Jim Meyering]

tags 45093 notabug
close 45093
stop

On Mon, Dec 7, 2020 at 9:11 AM Paul Eggert  wrote:
> On 12/6/20 8:23 PM, Robert S. Kissel wrote:
> > I'm pretty sure this is a bug in the Windoze port of head and tail,
>
> You should have better luck writing directly to the people who prepared that
> port, as they don't hang out on this mailing list and we largely don't worry
> about MS-Windows.

Thanks for replying, Paul. Marking this as "done" and not a bug.

bug#44235: [PATCH] dd: drop old workaround for lseek() bug in Linux kernel

[Jim Meyering]

On Mon, Oct 26, 2020 at 6:13 AM Pádraig Brady  wrote:
> On 26/10/2020 10:44, Kamil Dudka wrote:
> > The workaround triggers warnings with new kernel versions in case
> > a user does not have sufficient privileges for the MTIOCGET ioctl.
> >
> > * src/dd.c (skip_via_lseek): Drop wrapper function no longer needed.
> > (skip): Use lseek() directly.
> > (advance_input_after_read_error): Use lseek() directly.
> >
> > Reported-by: Nir Soffer
> > Bug: https://bugzilla.redhat.com/1876840
>
> To clarify, the kernel is warning, not dd.
> In general the kernel should be disallowing rather than warning I think.
> But yes this is old code that should no longer be needed.
> I'll apply the patch.

Thanks. Glad to see that code can be removed.
I still have that tape drive, but when I added that code (2000) may
have been the last time it was powered on.

bug#39364: [PATCH] rmdir: fix clobbered errno

[Jim Meyering]

On Mon, Feb 3, 2020 at 5:28 AM Pádraig Brady  wrote:
...
> Actually I think the key issue is not errno handling,
> but a logic error fixed with:
>
> @@ -102,7 +102,7 @@ ignorable_failure (int error_number, char const *dir)
> return (ignore_fail_on_non_empty
> && (errno_rmdir_non_empty (error_number)
> || (errno_may_be_empty (error_number)
> -  && is_empty_dir (AT_FDCWD, dir;
> +  && ! is_empty_dir (AT_FDCWD, dir;

Nice! Thanks for tracking that down. The patch looks great.
You might want to mention (in the log) the commit that introduced the
bug, since you already did the work to track it down:
v6.10-21-ged5c4e7

I preferred to require that for each NEWS-worthy bug fix, because
otherwise, it can be costly to re-derive or dig up in mail archives.

bug#39364: [PATCH] rmdir: fix clobbered errno

[Jim Meyering]

On Sun, Feb 2, 2020 at 5:11 AM Bernhard Voelker
 wrote:
> On 2020-02-02 07:32, Jim Meyering wrote:
> > FTR, here's a minimal test addition that exercises the bug. Succeeds
> > on 6.10, fails on 6.11:
>
> Minor tweak for the test ...
>
> -rmdir --ignore-fail-on-non-empty x/y && fail=1
> +returns_ 1 rmdir --ignore-fail-on-non-empty x/y || fail=1
>
>
> ... due to:
>
>   tests/rmdir/ignore.sh:36:rmdir --ignore-fail-on-non-empty x/y && fail=1
>   maint.mk: && fail=1 detected. Please use: returns_ 1 ... || fail=1
>   make: *** [cfg.mk:516: sc_prohibit_and_fail_1] Error 1

Thanks. Good point. Though note that I'm pretty sure that
prematurely-posted "test" is fundamentally inadequate. I haven't had
time to look more closely.

bug#39364: [PATCH] rmdir: fix clobbered errno

[Jim Meyering]

Nice find and thank you for the patch. That's a 12-year-old bug I introduced.
I confirm: with 6.10, running this:
  mkdir -p a/b && chmod a-w a && rmdir --ignore a/b
prints this and exits nonzero:
  rmdir: failed to remove `a/b': Permission denied

With 6.11 and newer, it silently succeeds.

On Fri, Jan 31, 2020 at 9:55 AM Pádraig Brady  wrote:
>
> On 31/01/2020 01:46, Matthew Pfeiffer wrote:
> > 'rmdir --ignore-fail-on-non-empty' would not report errors on non-empty
> > directories that fail for a different reason.
> > ---
> >   src/rmdir.c | 10 ++
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/src/rmdir.c b/src/rmdir.c
> > index c9f417957..7b253ab0d 100644
> > --- a/src/rmdir.c
> > +++ b/src/rmdir.c
> > @@ -133,18 +133,19 @@ remove_parents (char *dir)
> >   prog_fprintf (stdout, _("removing directory, %s"), quoteaf (dir));
> >
> > ok = (rmdir (dir) == 0);
> > +  int rmdir_errno = errno;
> >
> > if (!ok)
> >   {
> > /* Stop quietly if --ignore-fail-on-non-empty. */
> > -  if (ignorable_failure (errno, dir))
> > +  if (ignorable_failure (rmdir_errno, dir))
> >   {
> > ok = true;
> >   }
> > else
> >   {
> > /* Barring race conditions, DIR is expected to be a 
> > directory.  */
> > -  error (0, errno, _("failed to remove directory %s"),
> > +  error (0, rmdir_errno, _("failed to remove directory %s"),
> >quoteaf (dir));
> >   }
> > break;
> > @@ -233,12 +234,13 @@ main (int argc, char **argv)
> >
> > if (rmdir (dir) != 0)
> >   {
> > -  if (ignorable_failure (errno, dir))
> > +  int rmdir_errno = errno;
> > +  if (ignorable_failure (rmdir_errno, dir))
> >   continue;
> >
> > /* Here, the diagnostic is less precise, since we have no idea
> >whether DIR is a directory.  */
> > -  error (0, errno, _("failed to remove %s"), quoteaf (dir));
> > +  error (0, rmdir_errno, _("failed to remove %s"), quoteaf (dir));
> > ok = false;
> >   }
> > else if (remove_empty_parents)
> >
>
> This looks like a regression introduced in v6.10-21-ged5c4e7
> I.E. is_empty_dir() is globbering errno, and thus
> a non specific and non terminating warning is output,
> rather than a specific error, and exit.
>
> thanks,
> Pádraig
>
>
>

bug#38627: uniq -c gets wrong count with non-ascii strings

[Jim Meyering]

On Mon, Dec 16, 2019 at 1:41 AM Paul Eggert  wrote:
> On 12/15/19 11:40 AM, Roy Smith wrote:
> > With the following input:
> >
> >> $ cat x
> >> "ⁿᵘˡˡ"
> >> "ܥܝܪܐܩ"
> >
> >
> > Running "uniq -c" says there's two copies of the same line!
> >
> >> $ uniq -c x
> >>   2 "ⁿᵘˡˡ"
>
> Thanks for the bug report. I expect this is because GNU 'uniq' uses the
> equivalent of strcoll (locale-dependent comparison) to compare lines, whereas
> macOS 'uniq' uses the equivalent of strcmp (byte comparison). Since the two
> lines compare equal in your locale, GNU 'uniq' says there's just one line.
>
> The GNU 'uniq' behavior appears to be a consequence of this commit:
>
> commit 545c2323d493c7ed9c770d9b8e45a15db6f615bc
> Author: Jim Meyering 
> Date:   Fri Aug 2 14:42:37 2002 +
>
> with a change noted this way in NEWS:
>
> * uniq now obeys the LC_COLLATE locale, as per POSIX 1003.1-2001 TC1.
>
> However, the 2016 edition of POSIX removed mention of LC_COLLATE from 'uniq',
> and I expect this means that the 2002 commit should be reverted so that GNU
> 'uniq' behaves like macOS 'uniq' (a behavior that I think makes more sense 
> anyway).
>
> I'll CC: this email to Jim Meyering to see whether he has an opinion about 
> this.
>
> In the meantime you can work around the problem by using 'LC_ALL=C uniq' 
> instead
> of plain 'uniq' in your shell script.

Thanks for the report, Roy, and thanks Paul for diving in.
I confess I haven't done more than look at that old diff, but this
sure sounds like a bug we must fix, to get in line with the the much
more recent POSIX spec.

bug#36831: enhance 'directory not empty' message

[Jim Meyering]

On Sun, Jul 28, 2019 at 11:29 PM Assaf Gordon  wrote:
...
> What do others think? If this is a desired improvement, I'll finish the
> patch with news/tests/etc.
...
> [PATCH] mv: improve ENOTEMPTY/EEXIST error message
>
> Suggested by Alex Mantel  in
> https://bugs.gnu.org/36831 .
>
> $ mkdir A B B/A
> $ touch A/bar B/A/foo
>
> Before:
>
> $ mv A B
> mv: cannot move 'A' to 'B/A': Directory not empty
>
> After:
>
> $ mv A B
> mv: cannot move 'A' to 'B/A': Target directory not empty
>
> * src/copy.c (copy_internal): Add special handling for ENOTEMPTY/EEXIST.
> TODO: NEWS, tests.

I like it. Thank you.

bug#34239: build failure on Android, due to S_MAGIC_* symbols

[Jim Meyering]

On Tue, Jan 29, 2019 at 9:01 PM Pádraig Brady  wrote:
> On 28/01/19 19:19, Bruno Haible wrote:
> > Hi,
> >
> > Compiling coreutils on Android produces this error:
> >
> >   CC   src/tail.o
> > In file included from ../src/tail.c:63:
> > ../src/fs-is-local.h: In function 'is_local_fs_type':
> > ../src/fs-is-local.h:9: error: 'S_MAGIC_AAFS' undeclared (first use in this 
> > function)
> > ...
> > make[2]: *** [src/tail.o] Error 1
> >
> > The Android libc, Bionic, does not define any of these S_MAGIC_* symbols or
> > macros, even in the newest version [1].
> >
> > Can some #ifdef be used to avoid this build failure?
> > 'defined __ANDROID__' tests for Android.
> > 'defined __linux__' tests for Linux excluding Android.
> >
> > Bruno
> >
> > [1] https://android.googlesource.com/platform/bionic/
>
> Interesting. So inotify is supported on that android system.
> Our ifdefs were wrong anyway as we check for remoteness even if one disables 
> inotify.
> I.E. our build would have failed on standard linux if one explicitly disabled 
> inotify.
> I've fixed that up and added support for the android specific "sdcardfs" 
> which I found in:
> https://android.googlesource.com/kernel/common/+/android-mainline-tracking/include/uapi/linux/magic.h
>
> Proposed patch attached.

Looks all good. Thanks!

bug#30963: ls -fA -> still . and ..

[Jim Meyering]

On Tue, Mar 27, 2018 at 3:06 PM, Paul Eggert  wrote:
> On 03/27/2018 10:27 AM, Karl Berry wrote:
>>
>> ls -aA also shows . and ..; maybe it shouldn't?
>
> You're right, it shouldn't. This was a bug I introduced in 2004 and I think
> you're the first to report it (!). In my defense, it wasn't officially a bug
> until POSIX.1-2008 came out and specified that -a and -A should override
> each other. Anyway, thanks. I installed the attached patch.

Nice! Thank you both.

bug#29961: [PATCH] mv: document the missing atomicity of 'mv -n'

[Jim Meyering]

On Wed, Jan 3, 2018 at 8:27 AM, Kamil Dudka  wrote:
> On Wednesday, January 3, 2018 4:08:51 PM CET Pádraig Brady wrote:
>> Eep, Seems like we should use RENAME_NOREPLACE in this case,
>> rather than document the caveat?

Good catch/fix. Thanks to both of you.

bug#21760: timeout: Feature Request: --verbose ==> output if timeout

[Jim Meyering]

On Thu, Nov 23, 2017 at 2:35 PM, Pádraig Brady  wrote:
...
>> So I'm leaning towards supporting --verbose which would output something 
>> like:
>>
>>   timeout: aborting command 'blah' with signal SIGTERM
>>   timeout: aborting command 'blah' with signal SIGKILL

+  error (0, 0, _("aborting command %s with signal %s"),
+ quote (command), signame);

I like it, though maybe print "sending signal %s to command %s" ?
Sometimes (as above), the first attempt doesn't affect the process,
much less send SIGABRT, which one might infer from the use of "abort".

Also wording related, "the signal sent upon timeout" appears both in
code and in doc. I'd s/the/any/, since there can be more than one.

bug#29259: tail does not seek to the end of block device

[Jim Meyering]

On Mon, Nov 13, 2017 at 12:03 AM, Pádraig Brady  wrote:
> On 12/11/17 22:21, Pádraig Brady wrote:
>> On 12/11/17 21:52, Paul Eggert wrote:
>>> Why doesn't lseek work for this?
>>
>> Good call, it probably would.
>> Something like the following is more acceptable
>> since it adds very little complexity:
>
> Full patch attached with tests.

Nice work. I found nothing to improve.

bug#29164: Scratch this bug report

[Jim Meyering]

tags 29164 notabug
thanks

On Mon, Nov 6, 2017 at 12:21 AM, Thomas Deutschmann  wrote:
> please ignore this bug report. This is caused by Gentoo's sandbox in
> portage and no problem in coreutils. Sorry for wasting your time :/

Closing and marking as notabug

bug#29038: df hangs on fifos/named pipes

[Jim Meyering]

On Sun, Oct 29, 2017 at 3:34 PM, Pádraig Brady  wrote:
...
>> That was discovered by Martijn Dekker, CCed, when looking for a
>> portable way to identify the file system of an arbitrary file.
>
> Yes we shouldn't hang.
>
> RE side effects, open() is a fairly innocuous operation,
> and we expect stat() to have side effects to auto mount.
> (Note we avoid stat() with non specified arguments if possible due to this):
> https://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.27-92-ga19ff5d
>
> I suppose we could stat() and if that succeeds && !fifo, only then call 
> open() ?
> Patch to do that is attached.

Sounds good. Patch looks fine to me. Thanks!

bug#28859: Segmentation fault with NULL pointer dereference in 'stty'

[Jim Meyering]

On Tue, Oct 17, 2017 at 12:37 AM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 16/10/17 10:49, Jim Meyering wrote:
>> On Mon, Oct 16, 2017 at 2:30 AM, Pádraig Brady <p...@draigbrady.com> wrote:
>>> On 15/10/17 18:07, Jaeseung Choi wrote:
>>>> Dear GNU team,
>>>>
>>>> While testing coreutils for a research purpose, we found the following
>>>> crash in 'stty'. Running stty with the command-line "stty eol -F AA"
>>>> raises a crash as below. We did not change any terminal setting, and
>>>> believe the bug is irrelevant from any specific terminal
>>>> configuration.
>>>>
>>>> jason@ubuntu:~$ tar -xf coreutils-8.28.tar.xz
>>>> jason@ubuntu:~$ cd coreutils-8.28/
>>>> jason@ubuntu:~/coreutils-8.28$ mkdir obj
>>>> jason@ubuntu:~/coreutils-8.28$ cd obj
>>>> jason@ubuntu:~/coreutils-8.28/obj$ ../configure --disable-nls && make
>>>> ...
>>>> jason@ubuntu:~/coreutils-8.28/obj$ gdb ./src/stty -q
>>>> Reading symbols from ./src/stty...done.
>>>> (gdb) run eol -F AA
>>>> Starting program: /home/jason/coreutils-8.28/obj/src/stty eol -F AA
>>>>
>>>> Program received signal SIGSEGV, Segmentation fault.
>>>> set_control_char (info=0x40a6f8 <control_info+120>, info=0x40a6f8
>>>> <control_info+120>, mode=0x6103c0 , arg=0x0) at
>>>> ../src/stty.c:1695
>>>> 1695  else if (arg[0] == '\0' || arg[1] == '\0')
>>>> (gdb) x/i $rip
>>>> => 0x40387a <apply_settings+746>:   movzbl (%rbx),%r14d
>>>> (gdb) info reg rbx
>>>> rbx0x0  0
>>>> (gdb)
>>>>
>>>> We could reproduce the bug in coreutils from version 8.27 to 8.28.
>>>> Also, the bug was reproducible in both Ubuntu 16.04 and Debian 9.1.
>>>> But the stty program pre-built in Debian 9.1 did not crash because
>>>> currently 8.26 version is installed in Debian.
>>>
>>> This is actually an old bug which you can reproduce with -F /dev/tty.
>>> The attached should fix it up.
>>
>> Thank you!
>> If it's not too hard to determine, would you please mention in the log
>> the commit that introduced the bug?
>
> Updated patch attached. I mistakenly thought getopt would
> permute the argv so NULLs were at the end.  The attached
> caters for NULLs interspersed in the argv[].

Good catch!
One suggestion: indent the backslashes to column 72, e.g., with this patch:
diff --git a/src/stty.c b/src/stty.c
index 1a5c1e962..29f78375a 100644
--- a/src/stty.c
+++ b/src/stty.c
@@ -1089,11 +1089,11 @@ apply_settings (bool checking, const char *device_name,
 struct termios *mode, bool *speed_was_set,
 bool *require_set_attr)
 {
-#define check_argument(arg) \
-  if (k == n_settings - 1 || ! settings[k+1]) \
-{ \
-  error (0, 0, _("missing argument to %s"), quote (arg)); \
-  usage (EXIT_FAILURE); \
+#define check_argument(arg)\
+  if (k == n_settings - 1 || ! settings[k+1])  \
+{  \
+  error (0, 0, _("missing argument to %s"), quote (arg));  \
+  usage (EXIT_FAILURE);\
 }

   for (int k = 1; k < n_settings; k++)

bug#28859: Segmentation fault with NULL pointer dereference in 'stty'

[Jim Meyering]

On Mon, Oct 16, 2017 at 2:30 AM, Pádraig Brady  wrote:
> On 15/10/17 18:07, Jaeseung Choi wrote:
>> Dear GNU team,
>>
>> While testing coreutils for a research purpose, we found the following
>> crash in 'stty'. Running stty with the command-line "stty eol -F AA"
>> raises a crash as below. We did not change any terminal setting, and
>> believe the bug is irrelevant from any specific terminal
>> configuration.
>>
>> jason@ubuntu:~$ tar -xf coreutils-8.28.tar.xz
>> jason@ubuntu:~$ cd coreutils-8.28/
>> jason@ubuntu:~/coreutils-8.28$ mkdir obj
>> jason@ubuntu:~/coreutils-8.28$ cd obj
>> jason@ubuntu:~/coreutils-8.28/obj$ ../configure --disable-nls && make
>> ...
>> jason@ubuntu:~/coreutils-8.28/obj$ gdb ./src/stty -q
>> Reading symbols from ./src/stty...done.
>> (gdb) run eol -F AA
>> Starting program: /home/jason/coreutils-8.28/obj/src/stty eol -F AA
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> set_control_char (info=0x40a6f8 , info=0x40a6f8
>> , mode=0x6103c0 , arg=0x0) at
>> ../src/stty.c:1695
>> 1695  else if (arg[0] == '\0' || arg[1] == '\0')
>> (gdb) x/i $rip
>> => 0x40387a :   movzbl (%rbx),%r14d
>> (gdb) info reg rbx
>> rbx0x0  0
>> (gdb)
>>
>> We could reproduce the bug in coreutils from version 8.27 to 8.28.
>> Also, the bug was reproducible in both Ubuntu 16.04 and Debian 9.1.
>> But the stty program pre-built in Debian 9.1 did not crash because
>> currently 8.26 version is installed in Debian.
>
> This is actually an old bug which you can reproduce with -F /dev/tty.
> The attached should fix it up.

Thank you!
If it's not too hard to determine, would you please mention in the log
the commit that introduced the bug?

bug#28506: coreutils 8.28 test suite hangs on APFS filesystem

[Jim Meyering]

On Sun, Sep 24, 2017 at 10:34 AM, Jack Howarth
 wrote:
> On Sun, Sep 24, 2017 at 1:16 PM, Jack Howarth <
...

> Attached are the tests/touch/trailing-slash.log and
> tests/touch/trailing-slash.trs files generated from a build on an APFS
> volume running 10.13 in case you can identify why that test is failing.

That test is failing because your system allows "touch
symlink-to-file-specified-with-trailing-slash/" to succeed, e.g., here
is how it's supposed to work, but on your system touch (mistakenly)
succeeds:

$ : > k && ln -s k j && touch j/
touch: setting times of 'j/': Not a directory

When a non-directory name is specified with a trailing slash, many
interfaces are required by POSIX to fail with ENOTDIR. It looks like
one of those on your system goes ahead and performs the requested
operation as if that slash were not present.

We can probably teach gnulib to detect and work around this flaw.

bug#28506: coreutils 8.28 test suite hangs on APFS filesystem

[Jim Meyering]

On Wed, Sep 20, 2017 at 10:20 PM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 18/09/17 18:07, Jack Howarth wrote:
>> On Mon, Sep 18, 2017 at 7:40 PM, Jim Meyering <j...@meyering.net> wrote:
>>
>>> On Mon, Sep 18, 2017 at 4:26 PM, Jack Howarth
>>> <howarth.mailing.li...@gmail.com> wrote:
>>>> On Mon, Sep 18, 2017 at 5:08 PM, Jim Meyering <j...@meyering.net> wrote:
>>> ...
>>>>> Is there any chance your failing test was via a python2 framework? I'm
>>>>> asking (on Pádraig's behalf) because there is a known problem whereby
>>>>> SIGPIPE is mishandled in that case, and that might explain this
>>>>> failure, since the data-generation phase relies on SIGPIPE killing
>>>>> this test's "yes" command.
>>>>
>>>> I doubt it as the hang doesn't happen under 10.13 when run on a JHFS
>>>> formatted volume.
>>>
>>> How did you run the tests?
>>>
>>
>> Actually, I forgot to mention that the coreutils test suite hang only
>> occurred on the APFS volumes when the coreutils built against the gettext
>> and libiconv from fink. A build outside of fink which didn't build against
>> those packages didn't show the hang in the coreutils test suite. The fink
>> gettext and libiconv packages that I am using are those from...
>>
>> https://sourceforge.net/p/fink/package-submissions/4955/
>>
>> and
>>
>> https://sourceforge.net/p/fink/package-submissions/5004/
>>
>> which are both patched for the format string strictness in High Sierra. I
>> found that using --disable-nls in configuring coreutils was insufficient to
>> suppress the test suite hang which I assume is due to the presence of...
>>
>> #define HAVE_LIBINTL_H 1
>>
>> in the generated ./lib/config.h
>>
>> despite the presence of...
>>
>> /* #undef HAVE_DCGETTEXT */
>> /* #undef HAVE_GETTEXT */
>>
>> when --disable-nls is used so it still could be a Unicode related change in
>> APFS, no?
>>   Jack
>
> The libintl bit reminded me of 
> https://lists.gnu.org/archive/html/bug-gnulib/2014-10/msg00014.html
> I.E. on OSX enabling those libs creates implicit threads I think.
> Perhaps that's messing with SIGPIPE handling and only the implicit
> thread gets it, thus not killing the main yes(1) thread.
> However the yes(1) is also protected with a timeout(1) call.
> Perhaps timeout(1) is a silent noop. We should support OSX through 
> DYLD_INSERT_LIBRARIES,
> but perhaps there is something preventing that on your system?
> But then would the timeout tests fail. Could you check the timeout tests with:
>
>   make SUBDIRS=. TESTS=tests/misc/filter.sh check
>
> In any case we should protect calls to timeout(1) to ensure it's supported.
> The attached does that at least.

Good idea.
Do you think there should be a syntax-check rule to ensure that any
timeout-using test first calls require_timeout_? This makes me wonder
if we should make timeout a function that does that job (the first
time only), and then exec's the real timeout command.

bug#28506: coreutils 8.28 test suite hangs on APFS filesystem

[Jim Meyering]

On Mon, Sep 18, 2017 at 4:26 PM, Jack Howarth
<howarth.mailing.li...@gmail.com> wrote:
> On Mon, Sep 18, 2017 at 5:08 PM, Jim Meyering <j...@meyering.net> wrote:
...
>> Is there any chance your failing test was via a python2 framework? I'm
>> asking (on Pádraig's behalf) because there is a known problem whereby
>> SIGPIPE is mishandled in that case, and that might explain this
>> failure, since the data-generation phase relies on SIGPIPE killing
>> this test's "yes" command.
>
> I doubt it as the hang doesn't happen under 10.13 when run on a JHFS
> formatted volume.

How did you run the tests?

bug#28506: coreutils 8.28 test suite hangs on APFS filesystem

[Jim Meyering]

On Mon, Sep 18, 2017 at 1:18 PM, Jack Howarth
 wrote:
> The coreutils 8.28 release, when built on macOS 10.13 under the new APFS
> filesystem, produces a hang during the test suite run. The hang appears to
> occur in the execution of coreutils-8.28/tests/split/filter.sh at..
>
> + yes
> + head -n200K
> + split -b1G '--filter=head -c1 >/dev/null'
> + for mode in ''\'''\''' ''\''r/'\'''
> + FILE = -
>
> according to the filter.log generated from executing the section of
> split/filter.sh containing...
>
> yes | head -n200K | split -b1G --filter='head -c1 >/dev/null' || fail=1
>
> # Ensure that "endless" input is ignored when all filters finish
> for mode in '' 'r/'; do
>   FILE = '-'
>   if test "$mode" = ''; then
> FILE = 'zero.in'
> truncate -s10T "$FILE" || continue
>   fi
>   for N in 1 2; do
> rm -f x??.n || framework_failure_
> timeout 10 sh -c \
>   "yes | split --filter='head -c1 >\$FILE.n' -n $mode$N $FILE" || fail=1
> # Also ensure we get appropriate output from each filter
> seq 1 $N | tr '0-9' 1 > stat.exp
> stat -c%s x??.n > stat.out || framework_failure_
> compare stat.exp stat.out || fail=1
>   done
> done
>
> I haven't opened a radar report yet as the Apple engineers can't look
> directly at the source code for coreutils due to the GPLv3 licensing and
> the test suite seems to be tangled up with the makefiles making it
> impossible to extract a stand-alone test case reproducer to attach to a
> radar bug report.
>   Jack
> ps Again, the hang seems to occur at the tail end of the log after it
> emits...
>
> + FILE = -
>
> Any suggestions on how reduce this to a simpler test case? I would note
> that the new APFS filesystem produces a failure in the python test suite...
>
> https://bugs.python.org/issue31380
>
> which is due to APFS not allowing files to be created with filenames that
> contain unassigned codepoints in the Unicode 9.0 standard, whereas HFS+
> does. So perhaps the coreutils hang might be a similar issue?

Thank you for the testing and for the report.

Is there any chance your failing test was via a python2 framework? I'm
asking (on Pádraig's behalf) because there is a known problem whereby
SIGPIPE is mishandled in that case, and that might explain this
failure, since the data-generation phase relies on SIGPIPE killing
this test's "yes" command.

bug#28461: erreurs

[Jim Meyering]

On Sat, Sep 16, 2017 at 11:27 PM, Assaf Gordon  wrote:
...
> Attached an updated patch with all instances of 'parameter'
> changed to 'argument'.

Looks fine. Thank you!
I marked the issue as "done".

bug#28461: erreurs

[Jim Meyering]

On Fri, Sep 15, 2017 at 8:37 PM, Pádraig Brady  wrote:
...
> That's a good improvement!

Indeed!

> 
> I'd change:
>
> s/missing more parameters after/missing parameter after/

Or, perhaps use wording similar to what test does:

  ..._("missing argument after %s")

bug#28054: coreutils 8.27 test failure on x86_64-foxkit-linux-musl

[Jim Meyering]

On Sun, Aug 13, 2017 at 1:07 AM, Pádraig Brady  wrote:
> On 11/08/17 11:49, A. Wilcox wrote:
>
>> FAIL: tests/misc/csplit-io-err
>> ==
> This was due to an inconsistency in the errors output by seq.
> A fix for that buglet is attached.
>
>> FAIL: tests/misc/printf-surprise
>> 
> Depending on exit 141 couples the script to the value of SIGPIPE
> and to the shell implementation (ksh will return 269 here for e.g.).
> So I've attached a solution that should hopefully work in all situations.
>
>> FAIL: tests/misc/sort-debug-warn
>> 
> This was due to an assumption that for "missing" locales
> that sort would fail to C rather than C.UTF8.
> I've adjusted sort to not assume that in the attached.

Nice work. All three patches looks fine.  You might want to insert "in
https://bugs.gnu.org/28054; at the end of each log message.

Considering the use of "yes | :", even though there's an identical
prior use in tests/misc/seq-epipe.sh, I wondered if there exists a
shell/system where that would infloop. Maybe paranoid overkill, but
inserting a timeout would avoid the possibility.

bug#27368: Minor concern: Confusing tail warning

[Jim Meyering]

On Sat, Jun 17, 2017 at 3:57 PM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 17/06/17 14:30, Pádraig Brady wrote:
>> On 17/06/17 07:35, Jim Meyering wrote:
>>> In this new function, please move the declaration of "i" into the for-loop:
>>>
>>> +static bool
>>> +any_non_regular (const struct File_spec *f, size_t n_files)
>>> +{
>>> +  size_t i;
>>> +
>>> +  for (i = 0; i < n_files; i++)
>
> I did that in about 100 instances at:
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=1379bdc

Nice! Thank you.

bug#27368: Minor concern: Confusing tail warning

[Jim Meyering]

On Sat, Jun 17, 2017 at 1:32 AM, Pádraig Brady  wrote:
...
> Two proposed patches for this are attached.

Nice fixes. Thank you!

In the NEWS addition:

   tail -f will now exit immediately if the output is piped
   and the reader of the pipe terminates.

+  tail -f will no longer erroneously warn about being ineffective
+  when following a single tty, as the simple blocking loop used
+  is effective in this case.

Please don't use "will" in these blurbs.
I.e., use the present tense instead. Referencing the future makes
sense only now, prior to the release.

   tail -f now exits immediately when the output is piped
   and the reader of the pipe terminates.

   tail -f no longer erroneously warns about being ineffective
   when following a single tty, as the simple blocking loop used
   is effective in this case.

   tail -f /dev/tty is now supported by avoiding inotify when any
   non regular file is specified, as inotify is ineffective with these.
   [bug introduced with inotify support added in coreutils-7.5]

--
In this new function, please move the declaration of "i" into the for-loop:

+static bool
+any_non_regular (const struct File_spec *f, size_t n_files)
+{
+  size_t i;
+
+  for (i = 0; i < n_files; i++)

bug#25680: maint: tweaks so syntax tests pass for previous commit

[Jim Meyering]

On Wed, Feb 15, 2017 at 11:46 AM, Paul Eggert  wrote:
> I see some problems with this followup patch:
>
> http://git.savannah.gnu.org/cgit/coreutils.git/commit/?id=bd4bb42d65aac6591277066739ca42d1ddcc2d0e
>
> in that it will make force-link.c and force-link.h harder to move to gnulib.
> Is there some way that we can have the code pass syntax checks without tying
> it so tightly to coreutils?
>
> Also, why do the syntax checks require those redundant 'extern's? They're
> not that common in practice and I'd rather avoid them.

Hi Paul,
Those "extern" directives are the sole clue that these symbols are
deliberately given external scope. That clue is used by the
sc_tight_scope syntax check rule to ensure that no symbol is
accidentally made external.

If/when it moves to gnulib, those directives should be removed.

bug#25041: Bugs in TAC and TAIL for closed stdin

[Jim Meyering]

On Sun, Nov 27, 2016 at 7:40 AM, Pádraig Brady  wrote:
> I'll push the attached later

Thanks to both of you.

That patch looks fine, modulo a formatting nit: the second line is
indented one space too far:

+  f->ignore = ! (reopen_inaccessible_files
+  && follow_mode == Follow_name);

bug#25004: Bug in OD utility

[Jim Meyering]

On Wed, Nov 23, 2016 at 5:16 PM, Marcel Böhme  wrote:
> Hi Pádraig,
>
>> On 24 Nov 2016, at 8:45 AM, Pádraig Brady  wrote:
>>
>> I can't reproduce the issue here BTW with ASAN and running in a tight
>> loop for a few minutes.  So perhaps it has been fixed in glibc already?
>> I have glibc-2.22-10.fc23.x86_64
>> Depending on how widespread the issue is will determine if it's worth
>> adding the check to gnulib.
>
> I can reproduce the crash on Ubuntu 14.04 x86_64 with preinstalled od version 
> 8.21 and the version in trunk.
>
>> What libc are you using?
>
> $ /lib/x86_64-linux-gnu/libc.so.6
> GNU C Library (Ubuntu EGLIBC 2.19-0ubuntu6.9) stable release version 2.19, by 
> Roland McGrath et al.
> Copyright (C) 2014 Free Software Foundation, Inc.
...
> Compiled by GNU CC version 4.8.4.
...

Both gcc-4.8.4 and EGLIBC 2.19 are showing their age.
I too have failed to reproduce this. I used a Fedora 25 system, which
has glibc-2.24.

bug#25003: Bug in SPLIT utility

[Jim Meyering]

On Wed, Nov 23, 2016 at 4:21 PM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 23/11/16 22:16, Pádraig Brady wrote:
>> On 23/11/16 17:30, Jim Meyering wrote:
>>> On Wed, Nov 23, 2016 at 5:22 AM, Marcel Böhme <boehme.mar...@gmail.com> 
>>> wrote:
>>>> Dear all,
>>>>
>>>> We are running small 1h fuzzing sessions with AFLFast, a fork of AFL.
>>>> We’ll be reporting each found bug separately.
>>>>
>>>> On Coreutils v8.25 and trunk, the following input crashes.
>>>> Option -n was introduced with v8.8.
>>>>
>>>> $ ./split -n7/75 7
>>>> Segmentation fault
>>>>
>>>> ASAN says:
>>>> =
>>>> ==53143==ERROR: AddressSanitizer: negative-size-param: (size=-6)
>>>> #0 0x7f8820eb9a10 in memmove 
>>>> (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x62a10)
>>>> #1 0x404d12 in memmove /usr/include/x86_64-linux-gnu/bits/string3.h:57
>>>> #2 0x404d12 in bytes_chunk_extract ../src/split.c:987
>>>> #3 0x404d12 in main ../src/split.c:1625
>>>> #4 0x7f881fd9cf44 in __libc_start_main 
>>>> (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
>>>> #5 0x4064a9  
>>>> (/home/ubuntu/subjects/coreutils/obj-asan/src/split+0x4064a9)
>>>>
>>>> 0x7f8821f9a006 is located 2054 bytes inside of 135168-byte region 
>>>> [0x7f8821f99800,0x7f8821fba800)
>>>> allocated by thread T0 here:
>>>> #0 0x7f8820f193a8 in __interceptor_malloc 
>>>> (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc23a8)
>>>> #1 0x40ec88 in xmalloc ../lib/xmalloc.c:41
>>>>
>>>> SUMMARY: AddressSanitizer: negative-size-param 
>>>> (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x62a10) in memmove
>>>
>>> Thank you for the report.
>>> Would you please provide the contents of your file named "7"?
>>
>> That's immaterial I think. I can reproduce with:
>>   src/split -n2/3 /dev/null
>> I'll dig into these

Looks perfect.
Thanks!

bug#25003: Bug in SPLIT utility

[Jim Meyering]

On Wed, Nov 23, 2016 at 5:22 AM, Marcel Böhme  wrote:
> Dear all,
>
> We are running small 1h fuzzing sessions with AFLFast, a fork of AFL.
> We’ll be reporting each found bug separately.
>
> On Coreutils v8.25 and trunk, the following input crashes.
> Option -n was introduced with v8.8.
>
> $ ./split -n7/75 7
> Segmentation fault
>
> ASAN says:
> =
> ==53143==ERROR: AddressSanitizer: negative-size-param: (size=-6)
> #0 0x7f8820eb9a10 in memmove 
> (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x62a10)
> #1 0x404d12 in memmove /usr/include/x86_64-linux-gnu/bits/string3.h:57
> #2 0x404d12 in bytes_chunk_extract ../src/split.c:987
> #3 0x404d12 in main ../src/split.c:1625
> #4 0x7f881fd9cf44 in __libc_start_main 
> (/lib/x86_64-linux-gnu/libc.so.6+0x21f44)
> #5 0x4064a9  (/home/ubuntu/subjects/coreutils/obj-asan/src/split+0x4064a9)
>
> 0x7f8821f9a006 is located 2054 bytes inside of 135168-byte region 
> [0x7f8821f99800,0x7f8821fba800)
> allocated by thread T0 here:
> #0 0x7f8820f193a8 in __interceptor_malloc 
> (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc23a8)
> #1 0x40ec88 in xmalloc ../lib/xmalloc.c:41
>
> SUMMARY: AddressSanitizer: negative-size-param 
> (/usr/lib/x86_64-linux-gnu/libasan.so.3+0x62a10) in memmove

Thank you for the report.
Would you please provide the contents of your file named "7"?

bug#24874: dd: misleading parsing of hex numbers

[Jim Meyering]

On Fri, Nov 4, 2016 at 12:03 PM, Pádraig Brady  wrote:
> On 04/11/16 16:20, Pádraig Brady wrote:
>> On 04/11/16 11:19, Stephan Bauroth wrote:
>>> Dear coreutils team :)
>>>
>>> I encountered a buglike behaviour of dd when handling skip and count
>>> parameters that are encoded in hex and thus prefixed with 0x.
>>>
>>> dd is not able to parse them, which is OK but would be great if it would
>>> be, but, worse, reads 0xf00 as 0. It does that silently. While an
>>> enduser will immediately notice this on count, since nothing is copied,
>>> behaviour for skip looks ok. (In fact, I noticed this only because I
>>> hexdumped the result after hours of debugging)
>>>
>>> While it's OK that dd can't parse these numbers, maybe there should be a
>>> warning that 0x was found and interpreted as 0. Since a char like 'x' is
>>> invalid within a number that by definition has to be decimal, a warning
>>> should be fairly easy to implement.
>>>
>>> Of course, the ability to parse hex numbers in these parameters would be
>>> awesome :)
>>>
>>> regards and thanks for your continuing work,
>>> Stephan Bauroth
>>
>> Ouch. That's a real gotcha.
>> Note hex digits after the 0x are diagnosed, but not decimal digits:
>>
>>   $ dd skip=0x100 seek=0xf00
>>   dd: invalid number: ‘0xf00’
>>
>> Disallowing 0x... could definitely break backwards compat though.
>> Consider `for rec in 0 1 2; do dd skip=${rec}x1024...`
>>
>> I suppose we could output a warning to suggest using
>>   $(($rec * $size)) or 0${rec}x${size}
>> if that really is the intention?
>>
>> Given the warning workaround would be suggested in the message,
>> and that it's a relatively rare usage, a warning is probably appropriate 
>> here.
>> We already warn in dd for various usage.
>>
>> I'll fix that for the coming release.
>
> Patch attached.

Ouch, confusing indeed. I like your solution. One nit:

- dd if=/dev/null count=0x1 seek=0x1 skip=0x1 status=none 2>err
+ dd if=/dev/null count=0x1 seek=0x1 skip=0x1 status=none 2>err || fail=1

bug#24604: Add '--no-preserve-roots' flag to 'rm' for better safety

[Jim Meyering]

On Tue, Oct 4, 2016 at 5:54 AM, Pádraig Brady  wrote:
> On 04/10/16 12:38, Pádraig Brady wrote:
>> On 04/10/16 03:21, Mohammed Sadiq wrote:
>>> '--no-preserve-root' that can be used to ignore if the path is root when 
>>> using
>>> the 'rm' command.
>>>
>>> But as the most of the GNU commands accepts shortened flag as long as
>>> there is no ambiguity, this can be an issue too. So, 'rm --n' may have the
>>> same effect as 'rm --no-preserve-root'. There may be several users unaware
>>> of this feature which can cause several issues.
>>>
>>> 1. A cracker may be able to trick a user to bring a system down using
>>> '--n' flag.
>>> 2. A folder/file name like '--n' as an argument to 'rm' command may
>>> try to delete
>>> the whole files (in case a '/' too appears as an argument), and
>>> the user won't
>>> find a reason why it happened.
>>>
>>> One way to overcome this is set '--no-preserve-roots' too an alias for
>>> '--no-preserve-root'. This means that the user will have include the whole 
>>> flag
>>> to ignore root check (shortening will create an ambiguity).
>>
>> An interesting idea.
>> The main focus of the --no-preserve-root option is to protect against
>> accidental insertion of a space with `rm -rf blah /` or `rm -rf $blah/`.
>> With malicious arguments though one can obfuscate using shell quoting,
>> and the recent ls quoting changes are more general protection against that.
>> In saying that I don't see any issue with this, and there is a slight
>> increase in protection, so I'd be 60:40 for making this change.
>
> This would break scripts that used shortened --no-preserve for example,
> though that's quite unlikely to be used.
>
> Implementation is attached.

I too like the idea.
Did you consider this alternate implementation?

$ src/rm --no-preserve-root a
src/rm: cannot remove 'a': No such file or directory
[Exit 1]
$ src/rm --no-preserve-roo a
src/rm: you may not abbreviate the --no-preserve-root option
[Exit 1]
diff --git a/src/rm.c b/src/rm.c
index 13a5714..100d02e 100644
--- a/src/rm.c
+++ b/src/rm.c
@@ -287,6 +287,9 @@ main (int argc, char **argv)
   break;

 case NO_PRESERVE_ROOT:
+  if (! STREQ (argv[optind - 1], "--no-preserve-root"))
+error (EXIT_FAILURE, 0,
+   "you may not abbreviate the --no-preserve-root option");
   preserve_root = false;
   break;

bug#24232: [PATCH] ls: postpone installation of signal handlers

[Jim Meyering]

On Tue, Sep 6, 2016 at 10:05 AM, Paul Eggert  wrote:
> On 09/06/2016 08:59 AM, Pádraig Brady wrote:
>>
>> Will push later.
>
>
> Before pushing, can you please change the name of "sigs" back to "sig"? I
> prefer the old name, as "sig[i]" clearly means "signal i", whereas "sigs[i]"
> is more confusing. (This is one of my pet peeves about array names.) Thanks.

I agree.
A good argument for "why?" is that we optimize for readability of the
more frequent use with an index, sig[i], rather than for the sole
declaration, or the less frequent uses of the array name without an
index.

bug#24251: Potential cp bug: directories created with --parents and --no-preserve=mode retain original mode bits don't match ~umask

[Jim Meyering]

On Thu, Aug 18, 2016 at 5:57 AM, Pádraig Brady  wrote:
> On 17/08/16 12:42, Mark Mitchell wrote:
>> Hi,
>>
>> I'm writing to report a potential bug with cp.  I don't think the mode bits
>> always get properly set on directories created when using the --parents
>> option combined with --no-preserve=mode option.  I'm not sure what the
>> expected behaviour is supposed to be, but my assumption is that the created
>> directories would match process's ~umask (like mkdir -p).
>>
>> I've attached a simple script and its output to demonstrate the behaviour.
>> I've tested this on the most recent master branch of the coreutils git
>> repository (output of git describe is 'v8.25-55-gff2178b').
>>
>> Thanks for your efforts on these utilities, it is much appreciated :-).
>
> I agree that's unexpected, especially since cp -r --no-preserve=mode
> does not copy the permissions for created dirs.
>
> I think this may have changed with:
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v5.92-435-gfdffb6b
>
> I've adjusted things back in the attached.

That looks right. Thank you!

bug#23825: maint: avoid md5sum.c warning from bleeding-edge gcc's -Wstrict-overflow

[Jim Meyering]

On Thu, Jun 23, 2016 at 7:26 AM, Pádraig Brady  wrote:
> On 23/06/16 08:13, Paul Eggert wrote:
>> Incidentally, 'yes' has a different bug: it mishandles the case where
>> 'write' succeeds but returns a value less than the buffer size. I'll try
>> to look into that too. Simplest would be to use stdio (the comments
>> indicate this has performance issues but I don't know what they are,
>> anyway correctness trumps performance).
>
> Good spot on the yes(1) write(2) bug.
> Shouldn't impact too often due to smallish BUFSIZ,
> and subsequent writes catching ENOSPC,
> but definitely could cause issues.
>
> The attached should fix that up, and keep the same
> performance characteristics.

Hi Pádraig,
Thank you. That looks fine, modulo a formatting nit. I'd correct it like this:

 -const char* pwrite = buf
 +const char *pwrite = buf

Hi Paul,
Thank you, too.
I confirmed that all compile (using yesterday's gcc-7 snapshot) with
your patch. My only reservation is barely worth mentioning: the new
variable names, operandp and operand_lim are a bit too long for my
taste (at least in the files where all uses are very near the
definitions). On the other hand, they are also more readable than
alternatives that came to mind, and in some cases are used far enough
away from point of definition that one can justify that added length.

bug#23825: maint: avoid md5sum.c warning from bleeding-edge gcc's -Wstrict-overflow

[Jim Meyering]

Building with a recent gcc-7 failed, so I wrote the attached patch. I
think we'll never have 2^31 command line arguments, so "unsigned int"
will ll be ok:

maint: avoid md5sum.c warning from bleeding-edge gcc's -Wstrict-overflow

Avoid this warning/error from gcc:
  src/md5sum.c:870:3: error: assuming signed overflow does not occur \
  when simplifying conditional to constant [-Werror=strict-overflow]
* src/md5sum.c (main): Use an unsigned variable as the loop index,
rather than optind.
From 8912aa7c6dc7acb76d2de648e89098943bfe149a Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@fb.com>
Date: Wed, 22 Jun 2016 07:18:16 -0700
Subject: [PATCH] maint: avoid md5sum.c warning from bleeding-edge gcc's
 -Wstrict-overflow

Avoid this warning/error from gcc:
  src/md5sum.c:870:3: error: assuming signed overflow does not occur \
  when simplifying conditional to constant [-Werror=strict-overflow]
* src/md5sum.c (main): Use an unsigned variable as the loop index,
rather than optind.
---
 src/md5sum.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/md5sum.c b/src/md5sum.c
index 39132e3..fa2840b 100644
--- a/src/md5sum.c
+++ b/src/md5sum.c
@@ -867,9 +867,9 @@ main (int argc, char **argv)
   if (optind == argc)
 argv[argc++] = bad_cast ("-");

-  for (; optind < argc; ++optind)
+  for (unsigned int j = optind; j < argc; ++j)
 {
-  char *file = argv[optind];
+  char *file = argv[j];

   if (do_check)
 ok &= digest_check (file);
-- 
2.8.0-rc2

bug#23537: timeout test gets false-positive for duration of -1.189731495357231765e+4932

[Jim Meyering]

tags 23537 notabug
stop

On Sun, May 15, 2016 at 8:06 AM, Jim Meyering <j...@meyering.net> wrote:
> On Sun, May 15, 2016 at 4:21 AM, Pádraig Brady <p...@draigbrady.com> wrote:
>> Has up to date centos6 the bug?
>> I didn't see it with glibc-2.12-1.166.el6_7.7.x86_64
>
> Yes. I am surprised that you don't see it and I do:
>
>   $ rpm -q glibc
>   glibc-2.12-1.166.el6_7.7.x86_64
>   $ src/timeout 0.1 sleep 1.189731495357231765e+4932; echo $?
>   0

Pádraig guessed (correctly) that I'd updated coreutils' gnulib to
include a temporarily buggy xstrdod
(811b09243ca01827469827bf0973728a8619d249), but not yet past the fix
from a day or so later.
So this was all a false alert.

bug#23537: timeout test gets false-positive for duration of -1.189731495357231765e+4932

[Jim Meyering]

On Sun, May 15, 2016 at 4:21 AM, Pádraig Brady  wrote:
> Has up to date centos6 the bug?
> I didn't see it with glibc-2.12-1.166.el6_7.7.x86_64

Yes. I am surprised that you don't see it and I do:

  $ rpm -q glibc
  glibc-2.12-1.166.el6_7.7.x86_64
  $ src/timeout 0.1 sleep 1.189731495357231765e+4932; echo $?
  0

bug#23537: timeout test gets false-positive for duration of -1.189731495357231765e+4932

[Jim Meyering]

On systems with recent glibc, this abuse of timeout elicits the expected error:

  $ src/timeout -- -1.189731495357231765e+4932 sleep 0
  src/timeout: invalid time interval ‘-1.189731495357231765e+4932’
  Try 'src/timeout --help' for more information.

But with glibc-2.12's strtod, that input maps to a double-precision
value of 0 rather than to -inf, so timeout does this:

  $ src/timeout -- -1.189731495357231765e+4932 sleep 0; echo $?
  0

Similarly, the sleep.sh test fails because even without the leading "-",
that number ($LDBL_MAX) maps to 0:

  $ src/timeout 0.1 sleep 1.189731495357231765e+4932; echo $?
  0

which causes two tests to fail:

  tests/misc/timeout-parameters
  tests/misc/sleep

I see a couple of ways to avoid trouble.
Perhaps the most general is to make gnulib's strtod module detect and
compensate for these errors.
But that's CentOS6-era glibc, so maybe not worth it for such a corner case.

bug#23442: [PATCH] maint: avoid new warning from gcc (GCC) 7.0.0 20160503 (experimental)

[Jim Meyering]

On Thu, May 5, 2016 at 1:54 AM, Bernhard Voelker
<m...@bernhard-voelker.de> wrote:
> On 05/04/2016 05:08 PM, Jim Meyering wrote:
>> Subject: [PATCH] maint: avoid new warning from gcc (GCC) 7.0.0 20160503
>>  (experimental)
>>
>> * src/id.c (main): When configured with --enable-gcc-warnings and using
>> the very latest gcc built from git, building would fail with this:
>>   src/id.c:200:8: error: assuming signed overflow does not occur when \
>> simplifying conditional to constant [-Werror=strict-overflow]
>>  bool default_format = (just_user + just_group + just_group_list
>>   ^~
>> Rewrite to use bool-appropriate operators.
>
> Thanks, that looks much cleaner.

Thanks again. Pushed and closed.

bug#23442: [PATCH] maint: avoid new warning from gcc (GCC) 7.0.0 20160503 (experimental)

[Jim Meyering]

On Wed, May 4, 2016 at 12:40 AM, Bernhard Voelker
<m...@bernhard-voelker.de> wrote:
> On 05/04/2016 05:59 AM, Jim Meyering wrote:
>> -  bool default_format = (just_user + just_group + just_group_list
>> +  bool default_format = (0U + just_user + just_group + just_group_list
>>   + just_context == 0);
>
> These are all bool - wouldn't it be better to use boolean instead of
> arithmetical operators?

Thanks for the review. Indeed. Using "0U" there is a bit too much: a
hack upon the hack of using addition.

While using addition can be seen as slightly more readable (assuming
you know the idiom), or even better because the generated code is
jump-free, I would argue that this code should be readable, and that
whether there is a short-circuiting jump is irrelevant to the
performance of id.

While it is tempting to use "|" (the code generated by gcc-5.3 -O3 is
smaller and still jump-free: https://godbolt.org/g/zqMTtg), that still
feels dubious, especially when you remember that with gnulib, we may
still be simulating "bool" on some crufty systems.
So it seems best to use the bool-appropriate operators.

Here's the adjusted patch:
From 9a4df07016827fe130016b1c3a4adaf91c54c301 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@fb.com>
Date: Tue, 3 May 2016 20:56:20 -0700
Subject: [PATCH] maint: avoid new warning from gcc (GCC) 7.0.0 20160503
 (experimental)

* src/id.c (main): When configured with --enable-gcc-warnings and using
the very latest gcc built from git, building would fail with this:
  src/id.c:200:8: error: assuming signed overflow does not occur when \
simplifying conditional to constant [-Werror=strict-overflow]
 bool default_format = (just_user + just_group + just_group_list
  ^~
Rewrite to use bool-appropriate operators.
---
 src/id.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/id.c b/src/id.c
index 38844af..218ee5a 100644
--- a/src/id.c
+++ b/src/id.c
@@ -197,8 +197,10 @@ main (int argc, char **argv)
   if (just_user + just_group + just_group_list + just_context > 1)
 error (EXIT_FAILURE, 0, _("cannot print \"only\" of more than one choice"));

-  bool default_format = (just_user + just_group + just_group_list
- + just_context == 0);
+  bool default_format = ! (just_user
+   || just_group
+   || just_group_list
+   || just_context);

   if (default_format && (use_real || use_name))
 error (EXIT_FAILURE, 0,
-- 
2.8.0-rc2

bug#23073: wc reports wrong byte counts when using '--from-files0=-'

[Jim Meyering]

On Sun, Mar 20, 2016 at 5:59 PM, William R. Fraser  wrote:
> When wc gets its list of files by reading from stdin, using the argument
> '--from-files0=-', it reuses the same fstatus struct for each file.
>
> The problem is that the 'wc' function checks the 'failed' member of this
> struct and if it is <=0, it skips doing fstat on the file. The main loop
> doesn't reset this value between files, so only the first file has fstat
> done on it.
>
> This can result in the 'wc' function seeking past the end of subsequent
> files and then over-reporting their byte counts.
>
> See the attached patch, which resets the fstatus struct in between files
> when reading the file list from stdin.

Thank you for the patch and report for what looks like
a bug in code I wrote.

bug#22931: tests/split/filter.sh fails on an XFS file system

[Jim Meyering]

On Sun, Mar 6, 2016 at 7:36 PM, Jim Meyering <j...@meyering.net> wrote:
> The split/filter.sh test would fail like this:
>
>   $ make check TESTS=tests/split/filter.sh VERBOSE=yes SUBDIRS=.
>   + truncate -s9223372036854775807 zero.in
>   + timeout 10 sh -c 'split --filter="head -c1 >/dev/null" -n 1 zero.in'
>   split: zero.in: cannot determine file size: Value too large for
> defined data type
>
> That value is 2^63-1 (nearly 8 exabytes), and split.c
> explicitly handles a file of that size, because that is
> the size reported for /dev/zero on GNU/Hurd systems,
> according to the comment.
>
> This fixes the test not to trigger that work-around:
> [I'll update the commit log with the issue URL as soon as it's assigned]

No feedback, so I presume no objection.
Pushed with the mentioned commit-log addition.

bug#22931: tests/split/filter.sh fails on an XFS file system

[Jim Meyering]

On Sun, Mar 6, 2016 at 7:36 PM, Jim Meyering <j...@meyering.net> wrote:
> The split/filter.sh test would fail like this:
>
>   $ make check TESTS=tests/split/filter.sh VERBOSE=yes SUBDIRS=.
>   + truncate -s9223372036854775807 zero.in
>   + timeout 10 sh -c 'split --filter="head -c1 >/dev/null" -n 1 zero.in'
>   split: zero.in: cannot determine file size: Value too large for
> defined data type
>
> That value is 2^63-1 (nearly 8 exabytes), and split.c
> explicitly handles a file of that size, because that is
> the size reported for /dev/zero on GNU/Hurd systems,
> according to the comment.
>
> This fixes the test not to trigger that work-around:
> [I'll update the commit log with the issue URL as soon as it's assigned]

A possible addition is to #ifdef-out the offending code
in src/split.c, so that the original example works -- at least
on non-Hurd systems. One might argue that we should
make these tools work consistently across all systems,
but in a corner case like this, I have a slight preference
for keeping the usual-case code slightly cleaner, and
working with an edge-case file size like the one in this
example.

bug#22931: tests/split/filter.sh fails on an XFS file system

[Jim Meyering]

The split/filter.sh test would fail like this:

  $ make check TESTS=tests/split/filter.sh VERBOSE=yes SUBDIRS=.
  + truncate -s9223372036854775807 zero.in
  + timeout 10 sh -c 'split --filter="head -c1 >/dev/null" -n 1 zero.in'
  split: zero.in: cannot determine file size: Value too large for
defined data type

That value is 2^63-1 (nearly 8 exabytes), and split.c
explicitly handles a file of that size, because that is
the size reported for /dev/zero on GNU/Hurd systems,
according to the comment.

This fixes the test not to trigger that work-around:
[I'll update the commit log with the issue URL as soon as it's assigned]
From 889415ab359c943ee4c358f8c5fb07bca95a4ead Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyer...@fb.com>
Date: Sun, 6 Mar 2016 16:38:01 -0800
Subject: [PATCH] tests: avoid false-failure of split/filter.sh on XFS

* tests/split/filter.sh: Use OFF_T_MAX-1 rather than OFF_T_MAX
as the size of a test file, to avoid false failure on an XFS file
system (or any file system permitting a file of size OFF_T_MAX).
---
 tests/split/filter.sh | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tests/split/filter.sh b/tests/split/filter.sh
index a93008b..bd2f210 100755
--- a/tests/split/filter.sh
+++ b/tests/split/filter.sh
@@ -52,9 +52,13 @@ returns_ 1 split -n 1/2 --filter='true' /dev/null 2>&1 || fail=1
 # where they would result in a non zero exit from split.
 yes | head -n200K | split -b1G --filter='head -c1 >/dev/null' || fail=1

+# Do not use a size of OFF_T_MAX, since split.c applies a GNU/Hurd
+# /dev/zero workaround for files of that size.  Use one less:
+N=$(expr $OFF_T_MAX - 1)
+
 # Ensure that "endless" input is ignored when all filters finish
 timeout 10 sh -c 'yes | split --filter="head -c1 >/dev/null" -n r/1' || fail=1
-if truncate -s$OFF_T_MAX zero.in; then
+if truncate -s$N zero.in; then
   timeout 10 sh -c 'split --filter="head -c1 >/dev/null" -n 1 zero.in' || fail=1
 fi

-- 
2.7.2

bug#22909: [PATCH] test: Document that -a and -o are undesirable

[Jim Meyering]

On Fri, Mar 4, 2016 at 9:50 AM, Jim Meyering <j...@meyering.net> wrote:
> On Fri, Mar 4, 2016 at 9:12 AM, Pádraig Brady <p...@draigbrady.com> wrote:
>> On 04/03/16 08:55, Eric Blake wrote:
> ...
>>> +NOTE: Use of binary -a and -o create inherently ambiguous situations.
>
> I like that, too. Thanks. One nit: s/create/creates/

Or perhaps better/shorter still:

  Binary -a and -o are inherently ambiguous.

bug#22909: [PATCH] test: Document that -a and -o are undesirable

[Jim Meyering]

On Fri, Mar 4, 2016 at 9:12 AM, Pádraig Brady  wrote:
> On 04/03/16 08:55, Eric Blake wrote:
...
>> +NOTE: Use of binary -a and -o create inherently ambiguous situations.

I like that, too. Thanks. One nit: s/create/creates/

bug#22698: ls output changes considered unacceptable

[Jim Meyering]

On Wed, Feb 17, 2016 at 8:46 AM, Mike Hodson  wrote:
>>On Tue, Feb 16, 2016 at 6:45 AM, Bernhard Voelker  
>>wrote:
>>> On 02/16/2016 11:50 AM, Jason A. Donenfeld wrote:
>>> [...] We don't want those single quotes.
>>
>>
>> Who exactly is "we"?
>>
>> I can only speak for myself: I'm don't really care too much about
>> what output format is choosen.
>>
>> Have a nice day,
>> Berny
>>
>
> As part of this community who:
> 1: personally does not not particularly care about how 'ls' output looks
> 2: knows how to change it
> 3: but also knows how many people worldwide do not like such change,
> or may not appreciate the subtleties of the change, or know
> immediately how to change it if their linux distro hasn't already
> 'worked around this usability issue',
>
> I decided to take 10 minutes of my time to correlate a list of just
> who the "we" is:
>
> http://www.eenyhelp.com/bug-813164-fact-dangerous-help-215955262.html
> http://osdir.com/ml/general/2016-02/msg21069.html
> https://www.reddit.com/r/linux/comments/457ptk/ls_default_to_quotingshellescape_for_output_to/
> http://unix.stackexchange.com/questions/258679/why-is-ls-suddenly-surrounding-items-with-spaces-in-single-quotes
> http://linuxmd.net/forum/komandnaya-stroka-terminal/261-gnu-coreutils-8-25-ne-tak-strashen-kak-kazhetsya
> https://github.com/thestinger/termite/issues/293
> https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1394230.html
> 'revert'
> https://www.mail-archive.com/debian-bugs-dist@lists.debian.org/msg1394231.html
> 'ls -N not a solution'

Thanks for the summary.
However, bear in mind that many many people objected
to systemd, too, to the point of sending death threats to
its author. Many more people recognized that systemd is
great, yet did not send email -- and all major distros
have converted.

bug#22042: don't say K bytes on both head and tail's man pages

[Jim Meyering]

On Mon, Nov 30, 2015 at 6:52 AM, Pádraig Brady  wrote:
> On 29/11/15 23:35, Bernhard Voelker wrote:
>> On 11/29/2015 12:16 AM, Pádraig Brady wrote:
>>> I remember having slight reservations about K too.
>>> http://lists.gnu.org/archive/html/bug-coreutils/2009-05/msg00279.html
>>> Nothing much better comes to mind.
>>
>> Thanks.
>> In that discussion, also longer alternatives like COUNT or CNT were
>> mentioned.  If we decide to go that way, I'd favor NUM among those
>> suggestions, as it may also be used as is in translations.
>>
>> While at it, I'm wondering if it would make sense to emphasize
>> the optional plus ('+') for tail in the first column, similar as
>> for the minus ('-') for head (although only for 'man head', but
>> not in the Texi documentation):
>>
>>   $ src/head --help | grep -A2 -- '^[ ]*-[cn]'
>> -c, --bytes=[-]K print the first K bytes of each file;
>>with the leading '-', print all but the last
>>K bytes of each file
>> -n, --lines=[-]K print the first K lines instead of the first 10;
>>with the leading '-', print all but the last
>>K lines of each file
>>
>>   $ src/tail --help | grep -A1 -- '^[ ]*-[cn]'
>> -c, --bytes=Koutput the last K bytes; or use -c +K to output
>>bytes starting with the Kth of each file
>>   --
>> -n, --lines=Koutput the last K lines, instead of the last 10;
>>or use -n +K to output starting with the Kth
>>
>> Is there some GNU standard for this?  I couldn't find much.
>
> +1 for [+]NUM

Fine with me, except for "NUMth" or "NUM'th", which do not read well.
I guess we could change s/with the Kth/with line NUM/

bug#15604: [coreutils] [PATCH] md5sum: Add option to ignore non-existant files

[Jim Meyering]

On Mon, Nov 23, 2015 at 2:20 PM, Pádraig Brady  wrote:
>> I'll push a bit later today.
>
> Pushed at 
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.24-91-g9fd0662
> Marking http://bugs.gnu.org/15604 done

Given how this warns/fails when using --check does nothing,

  $ :|sha1sum --check
  sha1sum: 'standard input': no properly formatted SHA1 checksum lines found
  [Exit 1]

should using --check with --ignore-missing also warn/fail when it
verifies no checksum?

  $ :|sha1sum |sed s/-/no-such/ |sha1sum --check --ignore-missing; echo $?
  0

bug#15604: [coreutils] [PATCH] md5sum: Add option to ignore non-existant files

[Jim Meyering]

On Mon, Nov 23, 2015 at 5:24 PM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 23/11/15 16:05, Jim Meyering wrote:
>> On Mon, Nov 23, 2015 at 2:20 PM, Pádraig Brady <p...@draigbrady.com> wrote:
>>>> I'll push a bit later today.
>>>
>>> Pushed at 
>>> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.24-91-g9fd0662
>>> Marking http://bugs.gnu.org/15604 done
>>
>> Given how this warns/fails when using --check does nothing,
>>
>>   $ :|sha1sum --check
>>   sha1sum: 'standard input': no properly formatted SHA1 checksum lines found
>>   [Exit 1]
>>
>> should using --check with --ignore-missing also warn/fail when it
>> verifies no checksum?
>>
>>   $ :|sha1sum |sed s/-/no-such/ |sha1sum --check --ignore-missing; echo $?
>>   0
>
> It's a fair point, but I see the first error as verifying the
> checksum file itself, and so separate functionality.
>
> Related to this is outputting "MISSING" as well as "OK"
> unless --quiet is specified, though I thought the lack
> of "OK" if no files found would be enough indication
> of an issue in the normal usage?

I think a common expected usage of --ignore-missing would be
the case of an SHA1SUM file listing all possibly-verified files for
which it is common to verify only the one or two downloaded files.
In any invocation that ends up ignoring *all* file names, I would
want a loud warning and failure, to be sure that my eyes (and/or
any tool) notice something is wrong.

The absence of an "OK" is far easier to miss than a diagnostic.
At least a few are often expected to be missing, so I see little
value in emitting "MISSING" diagnostics.

bug#15604: [coreutils] [PATCH] md5sum: Add option to ignore non-existant files

[Jim Meyering]

On Mon, Nov 23, 2015 at 6:24 PM, Pádraig Brady <p...@draigbrady.com> wrote:
> On 23/11/15 16:41, Jim Meyering wrote:
>> I think a common expected usage of --ignore-missing would be
>> the case of an SHA1SUM file listing all possibly-verified files for
>> which it is common to verify only the one or two downloaded files.
>> In any invocation that ends up ignoring *all* file names, I would
>> want a loud warning and failure, to be sure that my eyes (and/or
>> any tool) notice something is wrong.
>>
>> The absence of an "OK" is far easier to miss than a diagnostic.
>> At least a few are often expected to be missing, so I see little
>> value in emitting "MISSING" diagnostics.
>
> Yes I agree. Thinking more, one could have a syntactically correct
> checksum file which is adjusted to comment out certain entries, and
> currently sha1sum etc. (with or without --ignore-missing)
> will error out unless something is verified:
>
> $ echo '#'|sha1sum --check --ignore-missing
> sha1sum: standard input: no properly formatted SHA1 checksum lines found
>
> So given that the existing functionality is to ensure something is verified,
> then --ignore-missing should be consistent.
>
> I'll push the attached a bit later.

Thank you.
That looks great.

Only suggestions are barely worth mentioning.
When negating, I'm pretty sure the grammar police suggest to use singular:

-error (0, 0, _("%s: no files were verified"),
+error (0, 0, _("%s: no file was verified"),

Also, please insert the comma in this log message sentence:

  * src/md5sum.c (digest_check): Update a matched_checksums bool upon
  matched checksum, and fail (loudly unless --status is specified)

bug#21356: BUG: split shorter version of '--numeric-suffixes' give error

[Jim Meyering]

On Mon, Aug 31, 2015 at 10:13 AM, Pádraig Brady  wrote:
> I'll apply the attached later.

Nice. Thanks to both of you.

bug#20998: Out of bounds global read in shred / genpattern()

[Jim Meyering]

On Mon, Jul 6, 2015 at 5:45 PM, Pádraig Brady p...@draigbrady.com wrote:
 On 07/07/15 00:29, Hanno Böck wrote:
 Hi,

 There is an out of bounds read error in the function genpattern() in
 shred (coreutils 8.23). This issue only appears randomly.

 To test:
 a) recompile coreutils 8.23 with address sanitizer: ./configure
 CFLAGS=-fsanitize=address -g LDFLAGS=-fsanitize=address; make
 b) create a test file: touch x
 c) run shred multiple times on it with -n 20:
 for i in $(seq 1 1000); do src/shred -n 20 x; done

 You will see the errors. Here's the output from Address Sanitizer:

 ==25808==ERROR: AddressSanitizer: global-buffer-overflow on address 
 0x00416628 at pc 0x4047a0 bp 0x7ffc99fee730 sp 0x7ffc99fee720
 READ of size 4 at 0x00416628 thread T0
...
 SUMMARY: AddressSanitizer: global-buffer-overflow src/shred.c:782 genpattern


 Nice one!

 It looks like the restriction to the k patterns available
 was lost with v5.92-1462-g65533e1 and that this should
 fix it up.

 diff --git a/src/shred.c b/src/shred.c
 index 63bcd6f..74f7ad9 100644
 --- a/src/shred.c
 +++ b/src/shred.c
 @@ -785,6 +785,7 @@ genpattern (int *dest, size_t num, struct randint_source 
 *s)
n--;
  }
p++;
 +  k--;

Nice one, indeed.  Thanks to both of you!

bug#20923: mgetgroups.c vs getgrouplist warning on OS X

[Jim Meyering]

On Tue, Jun 30, 2015 at 9:36 AM, Paul Eggert egg...@cs.ucla.edu wrote:
 Jim Meyering wrote:

 same result as before:

 OK, let's give up on this approach and try something more direct.  I
 installed the attached patch; does it work on OS X?

Perfect.
I made latest coreutils use latest from gnulib, and now it all compiles
warning-free there with --enable-gcc-warnings and gcc-6.x

Thank you!

bug#20923: mgetgroups.c vs getgrouplist warning on OS X

[Jim Meyering]

On Sun, Jun 28, 2015 at 11:48 PM, Paul Eggert egg...@cs.ucla.edu wrote:
 Jim Meyering wrote:

 I compiled the just-published snapshot on OS X configured with
 --enable-gcc-warnings, and saw this:

 lib/mgetgroups.c: In function 'mgetgroups':
 lib/mgetgroups.c:90:45: error: pointer targets in passing argument 3
 of 'getgrouplist' differ in signedness [-Werror=pointer-sign]
 ng = getgrouplist (username, gid, g, max_n_groups);


 Does the attached gnulib patch fix things for you on OS X?

 Its documentation lists OS X 10.11 (released this month), as I assume the
 incompatiblity is in the latest version of OS X and that Apple's never going
 to fix it

Hi Paul,
Thanks for writing that, but it did not help: the first variant compiled
just fine here (and probably everywhere),
so HAVE_GETGROUPLIST_WITH_INT was not defined.

The problem arises only when using --enable-gcc-warnings and the prototype-
required int* (type of argument 3) conflicts with the gid_t* argument.

bug#20923: mgetgroups.c vs getgrouplist warning on OS X

[Jim Meyering]

On Mon, Jun 29, 2015 at 2:01 PM, Jim Meyering j...@meyering.net wrote:
 On Mon, Jun 29, 2015 at 11:39 AM, Paul Eggert egg...@cs.ucla.edu wrote:
 Jim Meyering wrote:

 the first variant compiled
 just fine here (and probably everywhere),
 so HAVE_GETGROUPLIST_WITH_INT was not defined.


 Yes, well, it did work on GNU/Linux, which is what I tested it on.

 How about if we make the check pickier, so that it is likely to be triggered
 even without --enable-gcc-warnings, as in the attached?

 Thanks for another patch! Unfortunately, that still evokes only a
 warning at least
 with gcc 6.0.0 20150426.

Darn it. I see that I mistakenly pushed one of your patches
when I pushed the linkat.m4 fix, Paul. Sorry about that.
Happy to revert, if you'd like that. Let me know.

bug#20923: mgetgroups.c vs getgrouplist warning on OS X

[Jim Meyering]

On Mon, Jun 29, 2015 at 7:19 PM, Paul Eggert egg...@cs.ucla.edu wrote:
 Jim Meyering wrote:

 Darn it. I see that I mistakenly pushed one of your patches
 when I pushed the linkat.m4 fix, Paul. Sorry about that.
 Happy to revert, if you'd like that. Let me know.

 No problem.  I installed the attached further patch; does it fix things for
 you?  If we can't get this to work I can revert the whole thing.

Thanks.

I adjusted via the attached patch (the declaration
of getgrouplist is in unistd.h, and the then-block
should use gid_t), and tested: there was not even a
warning for the test program emitted by the then-block,
so same result as before:

  lib/mgetgroups.c: In function 'mgetgroups':
  lib/mgetgroups.c:70:32: error: pointer targets in passing argument 3
of 'getgrouplist' differ in signedness [-Werror=pointer-sign]
   #  define getgrouplist_gids(g) (g)
  ^


mgetgroups.m4.patch
Description: Binary data

bug#20923: mgetgroups.c vs getgrouplist warning on OS X

[Jim Meyering]

I compiled the just-published snapshot on OS X configured with
--enable-gcc-warnings, and saw this:

lib/mgetgroups.c: In function 'mgetgroups':
lib/mgetgroups.c:90:45: error: pointer targets in passing argument 3
of 'getgrouplist' differ in signedness [-Werror=pointer-sign]
   ng = getgrouplist (username, gid, g, max_n_groups);
 ^

bug#20536: avoid new gcc warning: ENOTSUP vs EOPNOTSUPP

[Jim Meyering]

On Sun, May 10, 2015 at 7:17 AM, Paul Eggert egg...@cs.ucla.edu wrote:
 Jim Meyering wrote:

 +  return err == EOPNOTSUPP
 +#if ENOTSUP != EOPNOTSUPP
 +|| err == ENOTSUP
 +#endif
 +;

 Would the following work instead?  It's a bit cleaner to avoid #if:

   return err == EOPNOTSUPP || (ENOTSUP != EOPNOTSUPP  err == ENOTSUP);

I confirmed that too avoids the warning, so I've pushed it in your name.

bug#20536: avoid new gcc warning: ENOTSUP vs EOPNOTSUPP

[Jim Meyering]

Building with very new gcc-from-git, I encountered 3 new warnings.
Here's a patch to address them:

Without this change, very recent gcc (e.g., version 6.0.0 20150509)
would print the following when configured with --enable-gcc-warnings:

  src/copy.c:165:30: error: logical 'or' of equal expressions \
[-Werror=logical-op]
 (errno == EOPNOTSUPP || errno == ENOTSUP || errno == ENOSYS))
   ^
* src/system.h (is_ENOTSUP): New function.
* src/copy.c (punch_hole): Use it.
* src/ls.c (errno_unsupported, gobble_file): Use it.


0001-build-avoid-a-warning-form-gcc-s-new-Wlogical-op.patch
Description: Binary data

bug#20354: [feature request] ln with command line arguments in reverse order

[Jim Meyering]

On Sun, Apr 19, 2015 at 8:27 AM, Ma Jiehong ma.jieh...@gmail.com wrote:
 The translation in the help message is not wrong, and TARGET and
 LINK_NAME are used.

 The issue simply is with the mental model made when approaching the ln
 command. In French, the usual way to say when creating a link is: Je vais
 créer une lien A pointant vers B, which can be literally translated as I
 will create a link named A pointing on B.

 This is wrong, as it should be more something like Je vais lier A et B
 would be better (I will link A and B), but A and B are implicitly already
 existing when thinking about this sentence.

 Checking the help message of ln clarifies this, but it's counter-intuitive
 in French, and most people tend to make a mistake in the order of argument
 of ln, and then re-do it with arguments in reverse order. (me first, even
 though I use the cli everyday...)

The issue is not French-specific.
There are two fundamentally different cases:
  - hard link: I will link A and B
  - symlinks: I will create a symlink B pointing at A

bug#20214: Nohup input redirection inconsistent with documentation

[Jim Meyering]

On Fri, Mar 27, 2015 at 7:14 PM, Jim Meyering j...@meyering.net wrote:
 On Fri, Mar 27, 2015 at 3:11 PM, Paul Eggert egg...@cs.ucla.edu wrote:
 Isaac Schwabacher wrote:

 This is confusing at best

 Yes, at the very least the documentation should be improved.  I installed
 the attached patch to try to do that.

 Is it really better for a read on stdin to fail with EBADF rather than
 simply returning EOF


 It depends on whether we want GNU nohup to be a universal donor or a
 universal acceptor.  Right now it's more the former (if a program works with
 GNU nohup it should be portable to other nohup platforms); a nohup that
 makes stdin read from /dev/null would be more accepting of badly-written
 code developed elsewhere. I suppose I could be talked into that,
 particularly given Matlab's misbehavior here.  Jim?

 My rationale (didn't check and assume it was I) was that it is
 better to fail in a way more likely to alert the incautious user
 that they have misused the tool, rather than to silently
 accept questionable usage.

 Considering it has been this way for 10 years, and has
 exposed real bugs in client code, I am inclined to prefer
 the existing behavior.

 Don't shoot the messenger?

Thinking about this some more, I conclude that history, and the
ability to expose a few misuses are perhaps not sufficient argument
for maintaining the status quo. So if you (Paul) want to flip nohup to
universal acceptor, I would not object.

bug#20214: Nohup input redirection inconsistent with documentation

[Jim Meyering]

On Fri, Mar 27, 2015 at 3:11 PM, Paul Eggert egg...@cs.ucla.edu wrote:
 Isaac Schwabacher wrote:

 This is confusing at best

 Yes, at the very least the documentation should be improved.  I installed
 the attached patch to try to do that.

 Is it really better for a read on stdin to fail with EBADF rather than
 simply returning EOF


 It depends on whether we want GNU nohup to be a universal donor or a
 universal acceptor.  Right now it's more the former (if a program works with
 GNU nohup it should be portable to other nohup platforms); a nohup that
 makes stdin read from /dev/null would be more accepting of badly-written
 code developed elsewhere. I suppose I could be talked into that,
 particularly given Matlab's misbehavior here.  Jim?

My rationale (didn't check and assume it was I) was that it is
better to fail in a way more likely to alert the incautious user
that they have misused the tool, rather than to silently
accept questionable usage.

Considering it has been this way for 10 years, and has
exposed real bugs in client code, I am inclined to prefer
the existing behavior.

Don't shoot the messenger?

bug#20114: tr does not support multibyte characters in the first argument

[Jim Meyering]

On Mon, Mar 16, 2015 at 5:15 AM, Pádraig Brady p...@draigbrady.com wrote:
...
 Yes you're right Bruno.
 Multi-byte support in coreutils in general has languished,
 but we hope to start improving support in the next major release (9?)
 after the current imminent 8.24 stable release.

 To that end I've put together a plan:
 http://www.pixelbeat.org/docs/coreutils_i18n/

Very nice plan!

bug#19969: problem: wc -c doesn't read actual # of bytes in file

[Jim Meyering]

On Mon, Mar 2, 2015 at 1:29 PM, Linda Walsh coreut...@tlinx.org wrote:


 Jim Meyering wrote:

 As root:
 # cd /proc
 # find -H [^0-9]* -name self -prune -o -name thread-self -prune -o -type
 f !
 -name kmsg ! -name kcore ! -name kpagecount ! -name kpageflags -print0|wc
 -c
 --files0-from=- |sort -n


 Thanks for the report.
 However, with wc from coreutils-8.23 and a 3.10 kernel, this is no
 longer an issue.

 ---

 with coreutils 8.23 from suse 13.2 and uname:

 Linux Ishtar 3.18.5-Isht-Van #1 SMP PREEMPT Wed Feb 4 14:50:44 PST 2015
 x86_64 x86_64 x86_64 GNU/Linux

 it is an issue.  All the /proc/sys entries are still 0.

 Here's the output (with a some lines elided)...


 0 mpt/summary
 0 net/netfilter/nfnetlink_log

Correction: I was using wc from upstream git (coreutils-8.23.141), so as
Pádraig says, it will be fixed in 8.24.

bug#19969: problem: wc -c doesn't read actual # of bytes in file

[Jim Meyering]

On Sat, Feb 28, 2015 at 12:59 AM, Linda Walsh coreut...@tlinx.org wrote:
 (coreutils-8.21-7.7.7)

 wc -c(bytes) doesn't seem to reliably read the number
 of bytes in a file.

 I was wanting to find out what the largest data-source
 files in '/proc' and '/sys' (didn't get around to trying
 /sys, since all the files under /proc/sys return 0 bytes.

 Note -- wc -l doesn't return '0' on the /proc/sys files.

 As root:
 # cd /proc
 # find -H [^0-9]* -name self -prune -o -name thread-self -prune -o -type f !
 -name kmsg ! -name kcore ! -name kpagecount ! -name kpageflags -print0|wc -c
 --files0-from=- |sort -n

Thanks for the report.
However, with wc from coreutils-8.23 and a 3.10 kernel, this is no
longer an issue.

bug#19503: most translations of proper names aren't being used

[Jim Meyering]

On Sun, Jan 4, 2015 at 7:53 AM, Pádraig Brady p...@draigbrady.com wrote:
...
 Also there is the more general point about how correct
 it is to attribute a program to author(s) in any case,
 as that tracked to a much more accurate level of detail
 by git blame etc.  Should we be removing output of
 author names at runtime completely?

We cannot do that blindly, since we lack version control history
from before 1992, which would make it appear that David J. MacKenzie
(who wrote many of these tools from scratch) contributed nothing.

bug#19503: most translations of proper names aren't being used

[Jim Meyering]

On Sun, Jan 4, 2015 at 9:43 AM, Pádraig Brady p...@draigbrady.com wrote:
...
 BTW, it might have been nice to have the initial git commits
 for these tools attributed to the original author. Hindsight and all that :)

It would have been nice, indeed.

When I agreed to do the job of maintaining the fileutils, textutils and
shellutils packages, I tried hard to find a CVS repository (no
dVCS existed back then), but as far as I could tell, if there had been
one, it was gone. It was only reluctantly that I resorted to putting
old tarballs on versioned release branches.  While djm was the primary
author for many tools, there were invariably commits by others, too, as
seen in old/*/ChangeLog*.  However, without some VCS files, it was not
feasible to attribute.  Even with ChangeLog+CVS, automated attribution
as I did for the glibc cvs-to-git conversion was nontrivial: most commits
were done by Ulrich, but ChangeLog usually gave the name of the Author,
and reliably mapping the cvs user-name or ChangeLog-attributed name to
a git Real Name/email pair took some work.

bug#19456: GNU coreutils - touch / add -v, --verbose option

[Jim Meyering]

On Sun, Dec 28, 2014 at 12:33 AM, Jari Aalto jari.aa...@cante.net wrote:
 It would be nice to see progress of touched files. Please
 add option[1]:

Hi Jari,
My preference is to avoid adding the --verbose option
to programs like touch.  Here is some explanation for
why I have seriously considered taking the relatively
drastic step of removing that option from the ch???
commands that currently accept it (chmod, chown,
chgrp):

https://lists.gnu.org/archive/html/coreutils/2010-08/msg00042.html

The case for touch is not quite the same, but the above
shows where my bias originates.

bug#19377: bug#19378: [PATCH 3/4] cat, chcon, chgrp, chmod, chown, cp, du, head: support wildcards on OS/2

[Jim Meyering]

On Mon, Dec 15, 2014 at 9:11 PM, KO Myung-Hun kom...@gmail.com wrote:
 Jim Meyering wrote:
 On Mon, Dec 15, 2014 at 8:35 PM, KO Myung-Hun kom...@gmail.com wrote:
 Paul Eggert wrote:
 KO Myung-Hun wrote:
   /* Redirection and wildcarding when done by the utility itself.
  Generally a noop, but used in particular for native VMS. */
   #ifndef initialize_main
 -# define initialize_main(ac, av)
 +# ifndef __OS2__
 +#  define initialize_main(ac, av)
 +# else

 What happened to VMS?  The comment doesn't seem to match the code, and

 I don't know of VMS. Do you mean to change comments for OS/2 ?

 this suggests that the code should be moved to wherever VMS does its thing.

 Where is it ? I didn't find initialize_main() or others for VMS.

 It was never defined in any version-controlled file,
 so please just update the comment, replacing VMS with OS/2.

 Ok. Fixed.

Thanks.
Pushed with minor grammar fixes in the commit log.

bug#19377: bug#19378: [PATCH 3/4] cat, chcon, chgrp, chmod, chown, cp, du, head: support wildcards on OS/2

[Jim Meyering]

On Mon, Dec 15, 2014 at 12:57 AM, Pádraig Brady p...@draigbrady.com wrote:
 On 15/12/14 01:15, KO Myung-Hun wrote:


 Pádraig Brady wrote:
 forcemerge 19378 19377
 stop

 On 14/12/14 03:47, KO Myung-Hun wrote:
 And ln,ls,mv,rm,tail.

 * src/cat.c (main): Expand wildcards on OS/2.
 * src/chcon.c (main): Likewise.
 * src/chgrp.c (main): Likewise.
 * src/chmod.c (main): Likewise.
 * src/chown.c (main): Likewise.
 * src/cp.c (main): Likewise.
 * src/du.c (main): Likewise.
 * src/head.c (main): Likewise.
 * src/ln.c (main): Likewise.
 * src/ls.c (main): Likewise.
 * src/mv.c (main): Likewise.
 * src/rm.c (main): Likewise.
 * src/tail.c (main): Likewise.

 Patches from coreutils 8.8 by Paul Smedley.

 diff --git a/src/cat.c b/src/cat.c
 index c7bb7e1..0138114 100644
 --- a/src/cat.c
 +++ b/src/cat.c
 @@ -544,6 +544,10 @@ main (int argc, char **argv)
bool show_tabs = false;
int file_open_mode = O_RDONLY;

 +#ifdef __OS2__
 +  _wildcard (argc, argv);
 +#endif
 +

 Interesing, the OS/2 shell doesn't doe the globbing.

 Ported unixy shells(sh) support it, but OS/2 default shell(CMD) does not.

 I'm wondering about the scalability of this.
 Are there any facilities for dealing with arbitrary numbers
 of files, like with xargs for example?

 No. It always processes all files.

 What are the practical limits of the number of files?

 It's up to a free memory.

 Does _wildcard() exit with an error in this case?


 Call exit(255) with printing an error message.


 While the adjustment is small, it would be better to avoid the ifdef in all 
 programs.
 I think there is a -Zwildcard option to auto enable for all programs?
 Also is there an option to disable this expansion at runtime
 (which should be documented if available).
 For example to allow deleting a file called '*', which seems like a more 
 likely
 occurrence on this platform.

It would be better still not to modify so many programs directly.
Can you instead add one occurrence of that ifdef in system.h,
to change the definition of the initialize_main macro that is
already used from every main program?

bug#19377: bug#19378: [PATCH 3/4] cat, chcon, chgrp, chmod, chown, cp, du, head: support wildcards on OS/2

[Jim Meyering]

On Mon, Dec 15, 2014 at 8:35 PM, KO Myung-Hun kom...@gmail.com wrote:
 Paul Eggert wrote:
 KO Myung-Hun wrote:
   /* Redirection and wildcarding when done by the utility itself.
  Generally a noop, but used in particular for native VMS. */
   #ifndef initialize_main
 -# define initialize_main(ac, av)
 +# ifndef __OS2__
 +#  define initialize_main(ac, av)
 +# else

 What happened to VMS?  The comment doesn't seem to match the code, and

 I don't know of VMS. Do you mean to change comments for OS/2 ?

 this suggests that the code should be moved to wherever VMS does its thing.

 Where is it ? I didn't find initialize_main() or others for VMS.

It was never defined in any version-controlled file,
so please just update the comment, replacing VMS with OS/2.

bug#19377: bug#19376: [PATCH 4/4] build: use -pi.bak instead of -pi

[Jim Meyering]

On Sun, Dec 14, 2014 at 5:25 AM, Pádraig Brady p...@draigbrady.com wrote:
 forcemerge 19376 19377
 stop

 On 14/12/14 03:47, KO Myung-Hun wrote:
 This fixes the following error.

 -
 Can't do inplace edit without backup.
 -

 * Makefile.am (dist-hook): Use -pi.bak instead of -pi.
 * bootstrap.conf (bootstrap_epilogue): Likewise.
 ---
  Makefile.am| 2 +-
  bootstrap.conf | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

 diff --git a/Makefile.am b/Makefile.am
 index fb4af27..371eb59 100644
 --- a/Makefile.am
 +++ b/Makefile.am
 @@ -105,7 +105,7 @@ BUILT_SOURCES = .version
  # See the rm_subst comment for details.
  dist-hook: gen-ChangeLog
   $(AM_V_GEN)echo $(VERSION)  $(distdir)/.tarball-version
 - $(AM_V_at)perl -pi -e '$(rm_subst)' $(distdir)/Makefile.in
 + $(AM_V_at)perl -pi.bak -e '$(rm_subst)' $(distdir)/Makefile.in

  gen_start_date = 2008-02-08
  .PHONY: gen-ChangeLog
 diff --git a/bootstrap.conf b/bootstrap.conf
 index c0b5f02..0baf455 100644
 --- a/bootstrap.conf
 +++ b/bootstrap.conf
 @@ -366,7 +366,7 @@ bootstrap_epilogue()
# Why?  That pipeline searches all files in $(top_srcdir), and if you
# happen to have large files (or apparently large sparse files), the
# first grep may well run out of memory.
 -  perl -pi -e 's/if LC_ALL=C grep .GNU .PACKAGE.*; then/if true; then/' \
 +  perl -pi.bak -e 's/if LC_ALL=C grep .GNU .PACKAGE.*; then/if true; then/' 
 \
  po/Makefile.in.in

# Install our git hooks, as long as cp accepts the --backup option,

 This will leave .bak files in place on all platforms which isn't ideal.
 Pity `perl -i` doesn't handle the platform differences transparently.
 Does sed -i behave better. That's less portable though could be tried
 and then fall back to perl -i.

Actually, neither of those uses of perl -pi is run by one who builds
from a distribution tarball.  Only people who build from git and who
run make dist will run those commands, so I think it is fine to
require a working version of perl for those uses.

i.e., I would prefer not to incur the cost of ugly work-around
changes here for what appears to be a very fringe platform.

bug#18621: [BUG] wc -c incorrectly counts bytes in /sys

[Jim Meyering]

On Tue, Oct 7, 2014 at 5:36 PM, Pádraig Brady p...@draigbrady.com wrote:
 On 10/08/2014 12:51 AM, Paul Eggert wrote:
 Paul Eggert wrote:

 The attached patch still needs a changelog entry and test cases.

 I wrote those up and pushed the attached patch; this should fix the bug so 
 I'm closing the bug report.

 I was just going through the patch as it happens, and I found no issues.
 I see the tac changes should also avoid erroneous errors if a file was 
 truncated while reading for example.
 I pushed a follow up to avoid some new syntax check errors.

 thanks!
 Pádraig.

I pushed a patch to avoid a new failiure of the
split/b-chunk.sh test on non-Linux.

bug#18621: [BUG] wc -c incorrectly counts bytes in /sys

[Jim Meyering]

On Fri, Oct 3, 2014 at 9:48 AM, Pádraig Brady p...@draigbrady.com wrote:
 On 10/03/2014 03:47 PM, George Shuklin wrote:
...
 I'm not sure where the above code comes from,
 by coreutils trunk has the same behavior with these files.
 We could avoid it with the following patch.
 Note in the case where real small files don't
 take up space in the file system, this will involve a redundant read,
 however that will only be the case for small files so shouldn't
 be problematic.

 thanks,
 Pádraig.

 diff --git a/src/wc.c b/src/wc.c
 index 1ff007d..bf1ce76 100644
 --- a/src/wc.c
 +++ b/src/wc.c
 @@ -235,6 +235,7 @@ wc (int fd, char const *file_x, struct fstatus *fstatus)
  fstatus-failed = fstat (fd, fstatus-st);

if (! fstatus-failed  S_ISREG (fstatus-st.st_mode)
 +   fstatus-st.st_blocks
 (current_pos = lseek (fd, 0, SEEK_CUR)) != -1
 (end_pos = lseek (fd, 0, SEEK_END)) != -1)
  {

That looks like a fine fix.
However, a similar issue affects tac, when its lseek-SEEK_END fails:

  $ tac /sys/kernel/vmcoreinfo
  tac: /sys/kernel/vmcoreinfo: read error: Inappropriate ioctl for device

bug#17833: coreutils 8.22 df

[Jim Meyering]

On Thu, Jul 10, 2014 at 4:28 PM, Pádraig Brady p...@draigbrady.com wrote:
 The attached should handle this by giving precedence
 to displaying real device nodes.

Thanks.  That looks fine.
I'd add an s to that NEWS entry: s/give/gives/

+  df now give precedence to displaying real device nodes in the presence of

bug#7320: [PATCH] 'id' prints incorrectly groups for the session

[Jim Meyering]

On Thu, Jun 26, 2014 at 3:23 AM, Pádraig Brady p...@draigbrady.com wrote:
 -g=$(id -u $NON_ROOT_USERNAME) || framework_failure_
 +u=$(id -u $NON_ROOT_USERNAME) || framework_failure_
 +g=u

This will work better :-)

   g=$u

bug#17495: chgrp: mention of being a member of the target group

[Jim Meyering]

Looks fine. Two nits barely worth mentioning: one in the .texi file:
  s/group in/group of/

one in the log:
  s/\*man/* man/

Also, in the relative formality of documentation,
it's slightly better to write It is than It's

bug#17590: [PATCH] build: libstdbuf.so: avoid new OS X link failure

[Jim Meyering]

On Mon, May 26, 2014 at 1:25 AM, Pádraig Brady p...@draigbrady.com wrote:
 On 05/25/2014 11:19 PM, Jim Meyering wrote:
 On Sun, May 25, 2014 at 1:31 PM, Pádraig Brady p...@draigbrady.com wrote:
 On 05/25/2014 08:48 PM, Jim Meyering wrote:
 Without the attached patch, I'd get this new link failure on OS X:

 Undefined symbols for architecture x86_64:
   _libintl_gettext, referenced from:
   _apply_mode in src_libstdbuf_so-libstdbuf.o
 ld: symbol(s) not found for architecture x86_64
 collect2: error: ld returned 1 exit status
 make[2]: *** [src/libstdbuf.so] Error 1

 Oh cool, I presume that's since I generalized the
 stdbuf enablement check that stdbuf is now built
 on Mac OS X. I presume it works too or you would
 have seen the test failure.

 Change looks good.

 Pushed.  Unfortunately, once past that link failure,
 the test of new-to-OSX stdbuf fails.  Here's stdbuf.log:

 That shows that the test is correct,
 and indicates that the buffering settings were ignored.

 I did a very quick search which suggests something
 like the attached might work (assuming the build params
 we hardcode for building the shared lib are OK).

You appear to have nailed it.  The patch looks fine, and
with that, the test now passes on OS X 10.8.5.

bug#17590: [PATCH] build: libstdbuf.so: avoid new OS X link failure

[Jim Meyering]

Without the attached patch, I'd get this new link failure on OS X:

Undefined symbols for architecture x86_64:
  _libintl_gettext, referenced from:
  _apply_mode in src_libstdbuf_so-libstdbuf.o
ld: symbol(s) not found for architecture x86_64
collect2: error: ld returned 1 exit status
make[2]: *** [src/libstdbuf.so] Error 1


0001-build-libstdbuf.so-avoid-new-OS-X-link-failure.patch
Description: Binary data

bug#17590: [PATCH] build: libstdbuf.so: avoid new OS X link failure

[Jim Meyering]

On Sun, May 25, 2014 at 1:31 PM, Pádraig Brady p...@draigbrady.com wrote:
 On 05/25/2014 08:48 PM, Jim Meyering wrote:
 Without the attached patch, I'd get this new link failure on OS X:

 Undefined symbols for architecture x86_64:
   _libintl_gettext, referenced from:
   _apply_mode in src_libstdbuf_so-libstdbuf.o
 ld: symbol(s) not found for architecture x86_64
 collect2: error: ld returned 1 exit status
 make[2]: *** [src/libstdbuf.so] Error 1

 Oh cool, I presume that's since I generalized the
 stdbuf enablement check that stdbuf is now built
 on Mac OS X. I presume it works too or you would
 have seen the test failure.

 Change looks good.

Pushed.  Unfortunately, once past that link failure,
the test of new-to-OSX stdbuf fails.  Here's stdbuf.log:


stdbuf.log
Description: Binary data

bug#17455: [PATCH] shred: fix overflow checking of command-line options

[Jim Meyering]

On Sat, May 10, 2014 at 11:42 AM, Paul Eggert egg...@cs.ucla.edu wrote:
 * src/shred.c (main): Limit -n (number of passes) value to
 ULONG_MAX, not to UINT32_MAX, since the vars are unsigned long.
 Limit the -s (file size) value to OFF_T_MAX.
 ---
  src/shred.c | 9 +
  1 file changed, 5 insertions(+), 4 deletions(-)

 diff --git a/src/shred.c b/src/shred.c
 index 607c6be..f4347e0 100644
 --- a/src/shred.c
 +++ b/src/shred.c
...
 @@ -1256,9 +1256,10 @@ main (int argc, char **argv)

  case 's':
{
 -uintmax_t tmp;
 -if (xstrtoumax (optarg, NULL, 0, tmp, cbBkKMGTPEZY0)
 -!= LONGINT_OK)
 +intmax_t tmp;
 +if ((xstrtoimax (optarg, NULL, 0, tmp, cbBkKMGTPEZY0)
 + != LONGINT_OK)
 +|| OFF_T_MAX  tmp)

Hi Paul,
The above makes it so shred now accepts a negative size.
Before, that would be diagnosed as invalid:

   $ shred -s-1 k
   shred: -1: invalid file size

With a size of -2, shred will write 64KB blocks forever -- or until it
runs out of space.

Here's a patch to fix that and to add a test covering that case:


0001-shred-don-t-infloop-upon-negative-size.patch
Description: Binary data

bug#16171: ptx: heap buffer overrun, when run with two file arguments

[Jim Meyering]

On Mon, Apr 28, 2014 at 6:52 AM, Pádraig Brady p...@draigbrady.com wrote:
...
 I see it here too (as the only failure in make check with -fsanitize=address

 The attached should address this.

Thanks for taking the time to address that.
The patch looks fine, modulo what looks like an accidentally-added
empty line all by itself in one chunk, just before this comment:

  /* Define the refence field, if any.  */

bug#17035: [PATCH] chmod -c -R produces errors with special permissions

[Jim Meyering]

On Tue, Mar 18, 2014 at 11:45 AM, Pádraig Brady p...@draigbrady.com wrote:
 diff --git a/tests/chmod/c-option.sh b/tests/chmod/c-option.sh
...
 +# This should never warn, but in did when special
 +# bits are set on b (the common case under test)
 +chmod -c -R g+w a 2err
 +test -s err  fail=1

Thanks for adding the test.
I have a small preference for using compare-vs-/dev/null rather than
test -s, since the former will print the unexpected output in the log,
while the latter won't. So maybe this instead?

  compare /dev/null err || fail=1

bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

[Jim Meyering]

On Fri, Mar 14, 2014 at 4:49 AM, Pádraig Brady p...@draigbrady.com wrote:
...
 Hence since c_f_m() can validly fail even with CAN_MISSING,
 I agree your patch is correct.

 Please push.

Done.

bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

[Jim Meyering]

On Sat, Feb 22, 2014 at 1:57 AM, Erik Bernstein e...@fscking.org wrote:
 Package: coreutils
 Version: 8.21-1
 Severity: normal

 Hi,

 when ln is run with --relative --symbolic and and empty string as the
 target, it ungracefully dies with a segmentation fault. The memory
 violation appears to happen in src/relpath.c:38 when the two input paths
 are checked for leading slashes:

   if ((path1[1] == '/') != (path2[1] == '/'))

 How to reproduce:
 [1] Open a terminal
 [2] run: ln -sr '' foobar

 Result: segmentation fault  ln -sr '' foobar
 Expected result: Some kind of error message
...

Thank you for the bug report!
That also affected the very latest code in git.
Here is a patch:
From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@fb.com
Date: Thu, 13 Mar 2014 17:05:04 -0700
Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

Prior to this change, ln -sr '' F would segfault, attempting
to read path2[1] in relpath.c's path_common_prefix function.
This problem arises whenever canonicalize_filename_mode returns
NULL.
* src/ln.c (convert_abs_rel): Call relpath only when
both canonicalize_filename_mode calls return non-NULL.
* tests/ln/relative.sh: Add a test to trigger this failure.
* THANKS.in: List reporter's name/address.
* NEWS (Bug fixes): Mention it.
Reported by Erik Bernstein in 739...@bugs.debian.org.
---
 NEWS |  2 ++
 THANKS.in|  1 +
 src/ln.c | 16 ++--
 tests/ln/relative.sh |  5 +
 4 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 62966b2..b3ad65c 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,8 @@ GNU coreutils NEWS-*- 
outline -*-
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]

+  ln -sr '' F no longer segfaults: now it fails with the expected diagnostic
+
   shuf --repeat no longer dumps core if the input is empty.
   [bug introduced with the --repeat feature in coreutils-8.22]

diff --git a/THANKS.in b/THANKS.in
index 561d18c..2670265 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -193,6 +193,7 @@ Eric G. Miller  e...@jps.net
 Eric Pementepeme...@northpark.edu
 Eric S. Raymond e...@snark.thyrsus.com
 Erik Bennettbenn...@cvo.oneworld.com
+Erik Bernstein  e...@fscking.org
 Erik Corry  e...@kroete2.freinet.de
 Felix Lee   f...@teleport.com
 Felix Rauch Valenti fra...@cse.unsw.edu.au
diff --git a/src/ln.c b/src/ln.c
index aab9cf2..6726699 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
   char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
   char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

-  /* Write to a PATH_MAX buffer.  */
-  char *relative_from = xmalloc (PATH_MAX);
-
-  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+  char *relative_from = NULL;
+  if (realdest  realfrom)
 {
-  free (relative_from);
-  relative_from = NULL;
+  /* Write to a PATH_MAX buffer.  */
+  relative_from = xmalloc (PATH_MAX);
+
+  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+{
+  free (relative_from);
+  relative_from = NULL;
+}
 }

   free (targetdir);
diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
index 7636695..e8c25dc 100755
--- a/tests/ln/relative.sh
+++ b/tests/ln/relative.sh
@@ -45,4 +45,9 @@ mkdir web
 ln -sr latest web/latest
 test $(readlink web/latest) = '../release2' || fail=1

+# Expect this to fail with exit status 1.
+# Prior to coreutils-8.23, it would segfault.
+ln -sr '' F
+test $? = 1 || fail=1
+
 Exit $fail
-- 
1.9.0.167.g384364b

bug#17010: Bug#739752: coreutils: ln segfaults when run with --relative and an empty target

[Jim Meyering]

On Thu, Mar 13, 2014 at 7:22 PM, Pádraig Brady p...@draigbrady.com wrote:
 On 03/14/2014 01:42 AM, Jim Meyering wrote:
 From a6d2db8b6dfe15344aba4aefe9545eb3a4876d45 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@fb.com
 Date: Thu, 13 Mar 2014 17:05:04 -0700
 Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

 Prior to this change, ln -sr '' F would segfault, attempting
 to read path2[1] in relpath.c's path_common_prefix function.
 This problem arises whenever canonicalize_filename_mode returns
 NULL.
 * src/ln.c (convert_abs_rel): Call relpath only when
 both canonicalize_filename_mode calls return non-NULL.
 * tests/ln/relative.sh: Add a test to trigger this failure.
 * THANKS.in: List reporter's name/address.
 * NEWS (Bug fixes): Mention it.
 Reported by Erik Bernstein in 739...@bugs.debian.org.

 We can amend with the now allocated:

   Fixes http://bugs.gnu.org/17010

Done.

 diff --git a/NEWS b/NEWS
...
 +  ln -sr '' F no longer segfaults: now it fails with the expected diagnostic

 Probably should add:

  [bug introduced with the --relative feature in coreutils-8.16]

Definitely.  Thanks.

 diff --git a/src/ln.c b/src/ln.c
 index aab9cf2..6726699 100644
 --- a/src/ln.c
 +++ b/src/ln.c
 @@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

 Interesting. So canonicalize_filename_mode() can fail in this case,
 even with CAN_MISSING. It's unexpected that c_f_m() sets errno=ENOENT
 when CAN_MISSING is set. I wonder should we change that instead
 in gnulib? With CAN_MISSING I would expect this function to work
 on arbitrary strings, including the empty string.

What would you have c_f_m(, CAN_MISSING) return?
I know of no absolute name corresponding to the dot-relative empty string.

 diff --git a/tests/ln/relative.sh b/tests/ln/relative.sh
 +# Expect this to fail with exit status 1.
 +# Prior to coreutils-8.23, it would segfault.
 +ln -sr '' F
 +test $? = 1 || fail=1

 Won't the ln succeed on FreeBSD as per:
 http://bugs.gnu.org/13447

You're right.  Good catch.  Adjusted as well.
Thanks for the speedy and thorough review.
Here's a revised patch:
From 0093ac8d57a0f1a16fd09d98f6a524dddb6053e7 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@fb.com
Date: Thu, 13 Mar 2014 17:05:04 -0700
Subject: [PATCH] ln: with -sr, don't segfault for a TARGET of ''

Prior to this change, ln -sr '' F would segfault, attempting
to read path2[1] in relpath.c's path_common_prefix function.
This problem arises whenever canonicalize_filename_mode returns
NULL.
* src/ln.c (convert_abs_rel): Call relpath only when
both canonicalize_filename_mode calls return non-NULL.
* tests/ln/relative.sh: Add a test to trigger this failure.
* THANKS.in: List reporter's name/address.
* NEWS (Bug fixes): Mention it.
Reported by Erik Bernstein in 739...@bugs.debian.org.
Fixes http://bugs.gnu.org/17010.
---
 NEWS |  3 +++
 THANKS.in|  1 +
 src/ln.c | 16 ++--
 tests/ln/relative.sh |  5 +
 4 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/NEWS b/NEWS
index 62966b2..35d48e5 100644
--- a/NEWS
+++ b/NEWS
@@ -25,6 +25,9 @@ GNU coreutils NEWS-*- 
outline -*-
   it would display an error, requiring --no-dereference to avoid the issue.
   [bug introduced in coreutils-5.3.0]

+  ln -sr '' F no longer segfaults.  Now works as expected.
+  [bug introduced with the --relative feature in coreutils-8.16]
+
   shuf --repeat no longer dumps core if the input is empty.
   [bug introduced with the --repeat feature in coreutils-8.22]

diff --git a/THANKS.in b/THANKS.in
index 561d18c..2670265 100644
--- a/THANKS.in
+++ b/THANKS.in
@@ -193,6 +193,7 @@ Eric G. Miller  e...@jps.net
 Eric Pementepeme...@northpark.edu
 Eric S. Raymond e...@snark.thyrsus.com
 Erik Bennettbenn...@cvo.oneworld.com
+Erik Bernstein  e...@fscking.org
 Erik Corry  e...@kroete2.freinet.de
 Felix Lee   f...@teleport.com
 Felix Rauch Valenti fra...@cse.unsw.edu.au
diff --git a/src/ln.c b/src/ln.c
index aab9cf2..6726699 100644
--- a/src/ln.c
+++ b/src/ln.c
@@ -149,13 +149,17 @@ convert_abs_rel (const char *from, const char *target)
   char *realdest = canonicalize_filename_mode (targetdir, CAN_MISSING);
   char *realfrom = canonicalize_filename_mode (from, CAN_MISSING);

-  /* Write to a PATH_MAX buffer.  */
-  char *relative_from = xmalloc (PATH_MAX);
-
-  if (!relpath (realfrom, realdest, relative_from, PATH_MAX))
+  char *relative_from = NULL;
+  if (realdest  realfrom)
 {
-  free (relative_from);
-  relative_from = NULL;
+  /* Write to a PATH_MAX buffer.  */
+  relative_from = xmalloc (PATH_MAX

bug#16578: Wish: Support for non-native endianness in od

[Jim Meyering]

On Sat, Feb 8, 2014 at 2:01 PM, Pádraig Brady p...@draigbrady.com wrote:
 +  if (input_swap  sizeof(T)  1)  \
 +{   \
 +  int j;\

The new patch looks complete.  Thanks to both of you.
One nit: please change the type of j here (identical in attached)
to be unsigned, to match that of the upper bound.

 +  union {   \
 +T x;\
 +char b[sizeof(T)];  \
 +  } u;  \
 +  for (j = 0; j  sizeof(T); j++)   \
 +u.b[j] = ((const char *) p)[sizeof(T) - 1 - j]; \

Re this function in the new test,

 +in_swapped() { printf '%s' $in | sed s/.\{$1\}/\\n/g | rev | tr -d 
 '\n'; }

That would be our first use of rev. Is it ubiquitous enough to depend on?

bug#16171: ptx: heap buffer overrun, when run with two file arguments

[Jim Meyering]

Hi,

I built like this using just-built 4.9.0 20131216
(but it probably would work as well with 4.8.x):

  make check AM_CFLAGS='-ggdb3 -static-libasan -fsanitize=address'
 AM_LDFLAGS='-fsanitize=address -static-libasan -lpthread -ldl'

and then I ran this,

  echo a  a  echo b  b 
  ./ptx -g1 -w1 a b 21 | asan_symbolize.py -d

and include its output below.
That output shows a heap-read overrun bug that arises
because ptx was designed to process only one input file, yet
was later extended to process more than, but without some
important adjustments.

The underlying problem is that swallow_file_in_memory (called from main)
is setting the contents of the global text_buffer for the first file,
then updating it (clobbering old value) for the second file.
Yet, some pointers to the initial buffer have been squirreled away
and later, one of them (keyafter) is presumed to point into
the new text_buffer, which it does not.  The subsequent
SKIP_WHITE_BACKWARDS use backs up cursor until it is goes
out of bounds.

=
==3156==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x6020e90f at pc 0x409612 bp 0x7fff67a426c0 sp 0x7fff67a426b8
READ of size 1 at 0x6020e90f thread T0
#0 0x409611 in generate_all_output /cu/src/ptx.c:1425
#1 0x7f3080a89994 in __libc_start_main
/home/aurel32/eglibc/eglibc-2.17/csu/libc-start.c:276
#2 0x409bb4 in _start ??:?

0x6020e90f is located 1 bytes to the left of 3-byte region
[0x6020e910,0x6020e913)
allocated by thread T0 here:
#0 0x4458e7 in __interceptor_malloc _asan_rtl_
#1 0x462c8b in fread_file /cu/lib/read-file.c:73

SUMMARY: AddressSanitizer: heap-buffer-overflow ??:0 ??
Shadow bytes around the buggy address:
  0x0c047fff9cd0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9ce0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c047fff9d10: fa fa fa fa fa fa fd fd fa fa 03 fa fa fa fd fd
=0x0c047fff9d20: fa[fa]03 fa fa fa 00 00 fa fa 04 fa fa fa 04 fa
  0x0c047fff9d30: fa fa fd fa fa fa fd fa fa fa 00 fa fa fa fd fa
  0x0c047fff9d40: fa fa 00 fa fa fa 00 fa fa fa fd fa fa fa fd fa
  0x0c047fff9d50: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fa
  0x0c047fff9d60: fa fa fd fa fa fa fd fa fa fa fd fa fa fa 00 fa
  0x0c047fff9d70: fa fa 00 fa fa fa 00 fa fa fa 00 fa fa fa 04 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:   00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:   fa
  Heap right redzone:  fb
  Freed heap region:   fd
  Stack left redzone:  f1
  Stack mid redzone:   f2
  Stack right redzone: f3
  Stack partial redzone:   f4
  Stack after return:  f5
  Stack use after scope:   f8
  Global redzone:  f9
  Global init order:   f6
  Poisoned by user:f7
  Contiguous container OOB:fc
  ASan internal:   fe
==3156==ABORTING

bug#15970: [Bug-tar] Crash in gettext() after fork() on Mac OS X

[Jim Meyering]

Hi Pádraig,

Thanks for dealing with that. Your patch looks correct.
Did you consider using inttostr in place of that first part of async_safe_error?

bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call

[Jim Meyering]

On Wed, Nov 20, 2013 at 4:03 PM, Bernhard Voelker
m...@bernhard-voelker.de wrote:
 On 11/20/2013 02:44 PM, Eric Blake wrote:
 On 11/19/2013 11:45 PM, Bernhard Voelker wrote:
 Maybe cannot remove directory is a bit weak - it's more like
 refusing to remove dot|dot-dot|root directory.

 Indeed, a clearer error message would be possible.

 What about the following?

   $ src/rm -r src/.
   src/rm: refusing to remove '.' or '..' directory: skipping 'src/.'

 I didn't want to explicitly mention POSIX here ... it's just to clarify
 that rm(1) does not swallow errors from the kernel like EPERM, etc.

 The texinfo file is enhanced in the patch below, too.
...
 ---
  doc/coreutils.texi |  3 ++-
  src/remove.c   | 12 

Thanks for doing that.
I was a little surprised to see an rm diagnostic change, but no
corresponding change in any test.  Perhaps an opportunity for
increased test coverage?

bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call

[Jim Meyering]

On Thu, Nov 21, 2013 at 8:18 AM, Bernhard Voelker
m...@bernhard-voelker.de wrote:
 On 11/21/2013 04:06 PM, Eric Blake wrote:

 Hard to say that it is considerable bloat without seeing a patch; we
 already know when the top-level arguments are directories thanks to 'rm
 -d'.


 Here's a draft - not tested more than this:

   $ mkdir -p /tmp/dir /tmp/dir/sub
   $ touch /tmp/dir/file /tmp/dir/sub/other

   $ src/rm -rv --child /tmp/dir
   removed ‘/tmp/dir/file’
   removed ‘/tmp/dir/sub/other’
   removed directory: ‘/tmp/dir/sub’
   src/rm: skipping ‘/tmp/dir’, due to --children-only

   $ src/rm -rv --child /tmp/dir
   src/rm: skipping ‘/tmp/dir’, due to --children-only

   $ src/rm -rv --child /tmp/dir/.
   src/rm: skipping ‘/tmp/dir/.’, due to --children-only


 Have a nice day,
 Berny

 From 03d58cc281c6155d50be9b770bbac7bf73cdaf92 Mon Sep 17 00:00:00 2001
 From: Bernhard Voelker m...@bernhard-voelker.de
 Date: Thu, 21 Nov 2013 17:11:27 +0100
 Subject: [PATCH] rm: add --children-only option

 FIXME
 ---
  src/mv.c |  1 +
  src/remove.c | 29 -
  src/remove.h |  4 
  src/rm.c |  9 +
  4 files changed, 38 insertions(+), 5 deletions(-)

 diff --git a/src/mv.c b/src/mv.c
 index 1cfcd82..36e70a4 100644
 --- a/src/mv.c
 +++ b/src/mv.c
 @@ -74,6 +74,7 @@ rm_option_init (struct rm_options *x)
  {
x-ignore_missing_files = false;
x-remove_empty_directories = true;
 +  x-children_only = false;
x-recursive = true;
x-one_file_system = false;

 diff --git a/src/remove.c b/src/remove.c
 index 3d386cf..eb05cb4 100644
 --- a/src/remove.c
 +++ b/src/remove.c
 @@ -439,8 +439,10 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const
 *x)
  {

/* POSIX says:
   If the basename of a command line argument is . or ..,
 - diagnose it and do nothing more with that argument.  */
 -  if (dot_or_dotdot (last_component (ent-fts_accpath)))
 + diagnose it and do nothing more with that argument.
 + FIXME: mention --children-only.  */
 +  if (! x-children_only
 +   dot_or_dotdot (last_component (ent-fts_accpath)))
  {
error (0, 0,

   _(refusing to remove %s or %s directory: skipping
 %s),
 @@ -468,9 +470,17 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const
 *x)

  if (s == RM_OK  is_empty_directory == T_YES)
{
 -/* When we know (from prompt when in interactive mode)
 -   that this is an empty directory, don't prompt twice.  */
 -s = excise (fts, ent, x, true);
 +if (FTS_ROOTLEVEL == ent-fts_level  x-children_only)
 +  {
 +error (0, 0, _(skipping %s, due to --children-only),
 +   quote (ent-fts_path));
 +  }
 +else
 +  {
 +/* When we know (from prompt when in interactive mode)
 +   that this is an empty directory, don't prompt twice.  */
 +s = excise (fts, ent, x, true);
 +  }
  fts_skip_tree (fts, ent);
}

 @@ -492,6 +502,15 @@ rm_fts (FTS *fts, FTSENT *ent, struct rm_options const
 *x)
  case FTS_NSOK: /* e.g., dangling symlink */
  case FTS_DEFAULT:  /* none of the above */
{
 +if (ent-fts_info == FTS_DP
 + x-children_only
 + FTS_ROOTLEVEL == ent-fts_level)
 +  {
 +mark_ancestor_dirs (ent);
 +error (0, 0, _(skipping %s, due to --children-only),
 +   quote (ent-fts_path));
 +return RM_OK;
 +  }
  /* With --one-file-system, do not attempt to remove a mount point.
 fts' FTS_XDEV ensures that we don't process any entries under
 the mount point.  */
 diff --git a/src/remove.h b/src/remove.h
 index 9ac54d4..248a470 100644
 --- a/src/remove.h
 +++ b/src/remove.h
 @@ -52,6 +52,10 @@ struct rm_options
/* If true, remove empty directories.  */
bool remove_empty_directories;

 +  /* If true (and the -r option is also specified), remove all children
 + of directory arguments, yet retaining the directory itself.  */
 +  bool children_only;
 +
/* Pointer to the device and inode numbers of '/', when --recursive
   and preserving '/'.  Otherwise NULL.  */
struct dev_ino *root_dev_ino;
 diff --git a/src/rm.c b/src/rm.c
 index 7a51eef..0634855 100644
 --- a/src/rm.c
 +++ b/src/rm.c
 @@ -51,6 +51,7 @@ enum
ONE_FILE_SYSTEM,
NO_PRESERVE_ROOT,
PRESERVE_ROOT,
 +  CHILDREN_ONLY,
PRESUME_INPUT_TTY_OPTION
  };

 @@ -78,6 +79,7 @@ static struct option const long_opts[] =

{recursive, no_argument, NULL, 'r'},
{dir, no_argument, NULL, 'd'},
 +  {children-only, no_argument, NULL, CHILDREN_ONLY},
{verbose, no_argument, NULL, 'v'},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
 @@ 

bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call

[Jim Meyering]

On Thu, Nov 21, 2013 at 5:39 AM, Eric Blake ebl...@redhat.com wrote:
 On 11/21/2013 12:12 AM, Bernhard Voelker wrote:
...
 But that's not what Linda is asking for.  She is not asking to pull .
 out of under her feet.  Instead, she wants a command that will
 recursively remove the children of ., but then leave . itself
 unremoved (whether by virtue of the fact that rmdir(.) must fail and
 so the overall rm command fails, or by explicitly skipping the attempt
 to rmdir(.) and letting rm succeed).  Right now, the nanny rule of
 POSIX is preventing the recursion, so you have to use contortions such
 as the POSIX 'find . -depth ! -name . -exec rm {} +'.  So I think it IS
 useful to add an option that forces 'rm -r' to bypass the nanny rule and
 recurse on ..

 Maybe naming it --no-preserve-dot is wrong.  Maybe a better name is 'rm
 -r --children-only .'.  At which point, I would much rather see us skip
 the rmdir(.) in order to allow rm to succeed.  And it would also work
 even for non-dot situations: 'rm -r --children-only dir'.  In other
 words, I _do_ see what Linda is asking for, and think it is worth providing.

Thanks for clarifying, Eric.  I am open to the idea.  Amusingly,
when I first read your suggestion for --children-only, I thought,
That's backwards, shouldn't it be '--adults-only' to go with the
no-nanny semantics? Of course, children refers to tree parent/child
relationship, so would make sense in the manual, where there's no
talk of removing this nanny-like safeguard.  But then I wondered about
non-directory arguments, and how the implied semantics of --children-only
applies (or not) to them.  I don't particularly like the idea of adding
the long-named --(no-)?preserve-dot-or-dot-dot, because that would
render ambiguous any existing use of --(no-)?preserve- and all shorter
abbreviations of --(no-)?preserve-root.