bug#6366: join can't join on numeric fields

[Jim Meyering]

Alex Shinn wrote:

 2010/6/8 Pádraig Brady p...@draigbrady.com:
 On 07/06/10 06:19, Alex Shinn wrote:

 Ideally join should be able to handle files sorted in any order
 that sort provides, but as a bare minimum it should at least
 be able to join files sorted on numeric fields.

 Well if there were no aliases in the numbers, you could always
 sort the output numerically after the join if it was important.

 By first sorting lexicographically, you mean?
 In the use case I had, the data was already sorted
 numerically.  So whenever I want to join two files,
 currently I have to do:

   sort file1  file1.tmp
   sort file2  file2.tmp
   join file1.tmp file2.tmp | sort -n  out
   rm -f file1.tmp file2.tmp

 instead of just

   join -n file1 file2  out

 In the small tools philosophy you want to avoid adding
 redundancy, but in this case join isn't doing the same
 thing as sort, it's just working with it better.  Not to mention
 the fact that sort is an expensive operation to have to
 perform multiple times, not just an extra O(n) filter
 to throw in the middle of a pipeline.

 However if you wanted to join 01 and 1 then your patch is required.
 Are numeric aliases common enough to warrant this? I think so.

 Leading zeros may not be so common, but don't forget
 1.0 and 1 or 1e2 and 100 and 100.0, etc.

 I'd use -g, --general-numeric to correspond with `sort`.

 Yes, that's probably better.

There may be a fly in the ointment.

When comparing floating point numbers how would join measure equality?
Should it consider 1.001e2 to be equal to 100.0 ?
What if the maximum precision available does not
allow us to distinguish those two values?

What about -0 and 0? (with IEEE 754, they'll compare equal)

bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[Jim Meyering]

Paul Eggert wrote:
 Jeff Liu and Jim Meyering wrote:
 diff --git a/src/fiemap.h b/src/fiemap.h
 new file mode 100644
 index 000..d33293b
 --- /dev/null
 +++ b/src/fiemap.h

Hi Paul,

Thanks for the review.

 Why is this file necessary?  fiemap.h is included only if it exists,
 right?  Shouldn't we just use the kernel's fiemap.h rather than
 copying it here and assuming kernel internals?

The ioctl is available/usable in 2.6.27 and newer that do not publish
this file.  For example, it's in F13's (2.6.33's) /usr/include/linux/fiemap.h,
as well as the one in debian unstable's 2.6.32, but probably
not in much older kernels.

Hmm..  I see that it's available even in F11's 2.6.30.9-x

It would be better to include linux/fiemap.h when present,
else our copy of that header.  Then, eventually, the else
clause will become unused.  Note that even on newer kernels,
the linux/* headers are optional.

Eventually we'll have a hard requirement on kernel headers --
at least when building against a linux kernel.

  if (lseek (src_fd, ext_logical, SEEK_SET)  0LL)

 For this sort of thing, please just use 0 rather than 0LL.
 0 is easier to read and it has the same effect here.

Included in the patch below.

  char buf[buf_size];

 This assumes C99, since buf_size is not known at compile time.
 Also, isn't there a potential segmentation-violation problem
 if buf_size is sufficiently large?

 More generally, since the caller is already allocating a buffer of the
 appropiate size, shouldn't we just reuse that buffer, rather than
 allocating a new one?  That would avoid the problems of assuming
 C99 and possible segmentation violations.

Good point.  Thanks.
We can definitely avoid that allocation.
Do you feel like writing the patch?

I've just pushed this series to a branch, fiemap-copy,
so others can follow along and contribute more easily.

   char fiemap_buf[4096];
   struct fiemap *fiemap = (struct fiemap *) fiemap_buf;
   struct fiemap_extent *fm_ext = fiemap-fm_extents[0];
   uint32_t count = ((sizeof fiemap_buf - sizeof (*fiemap))
 / sizeof (struct fiemap_extent));

 This code isn't portable, since fiemap_buf is only char-aligned, and
 struct fiemap may well require stricter alignment.  The code will work
 on the x86 despite the alignment problem, but that's just a happy
 coincidence.

 A lesser point: the code assumes that 'struct fiemap' is sufficiently
 small (considerably less than 4096 bytes in size); I expect that this
 is universally true but we might as well check this assumption, since
 it's easy to do so without any runtime overhead.

 So I propose something like this instead:

union { struct fiemap f; char c[4096]; } fiemap_buf;
struct fiemap *fiemap = fiemap_buf.f;
struct fiemap_extent *fm_ext = fiemap-fm_extents[0];
enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_ext };
verify (count != 0);

I've done this in your name:

From fffa2e8661a27978927fcc8afb6873631a753292 Mon Sep 17 00:00:00 2001
From: Paul Eggert egg...@cs.ucla.edu
Date: Wed, 9 Jun 2010 08:15:07 +0200
Subject: [PATCH] copy.c: ensure proper alignment of fiemap buffer

* src/copy.c (fiemap_copy): Ensure that our fiemap buffer
is large enough and well-aligned.
Replace 0LL with equivalent 0 as 3rd argument to lseek.
---
 src/copy.c |   15 ---
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index f629771..27e083a 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -171,11 +171,12 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size,
  char const *dst_name, bool *normal_copy_required)
 {
   bool last = false;
-  char fiemap_buf[4096];
-  struct fiemap *fiemap = (struct fiemap *) fiemap_buf;
+  union { struct fiemap f; char c[4096]; } fiemap_buf;
+  struct fiemap *fiemap = fiemap_buf.f;
   struct fiemap_extent *fm_ext = fiemap-fm_extents[0];
-  uint32_t count = ((sizeof fiemap_buf - sizeof (*fiemap))
-/ sizeof (struct fiemap_extent));
+  enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_ext };
+  verify (count != 0);
+
   off_t last_ext_logical = 0;
   uint64_t last_ext_len = 0;
   uint64_t last_read_size = 0;
@@ -185,7 +186,7 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size,
   /* This is required at least to initialize fiemap-fm_start,
  but also serves (in mid 2010) to appease valgrind, which
  appears not to know the semantics of the FIEMAP ioctl. */
-  memset (fiemap_buf, 0, sizeof fiemap_buf);
+  memset (fiemap_buf, 0, sizeof fiemap_buf);

   do
 {
@@ -220,13 +221,13 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size,
   off_t ext_logical = fm_ext[i].fe_logical;
   uint64_t ext_len = fm_ext[i].fe_length;

-  if (lseek (src_fd, ext_logical, SEEK_SET)  0LL)
+  if (lseek (src_fd, ext_logical, SEEK_SET)  0)
 {
   error (0, errno, _(cannot lseek %s), quote (src_name));
   return false;
  

bug#6381: [PATCH] Add digit grouping option for file sizes in ls

[Sam Bobroff]

Hello,

Please excuse me if I'm not going about this the right way but I would
like to offer a tiny patch for ls that adds an option to turn on digit
grouping for file sizes in the long output format.

(The support for displaying digit grouping is already present, all that
is required is an option for turning it on.)

I find it a very nice way to view file sizes because it makes it easy to
read the size of large files (like -h) but it does not hide any
information (unlike -h).

For example:

$ ls -l bigfile
-rw-r--r-- 1 samb samb 2146034154 2010-06-08 14:55 big.log

$ ls -ly bigfile
-rw-r--r-- 1 samb samb 2,146,034,154 2010-06-08 14:55 big.log

(The patch is against the latest git checkout but I have only actually
compiled it against the latest .tar release due to dependencies.)

Peace out,
Sam Bobroff.
diff --git a/src/ls.c b/src/ls.c
index 9549130..e5b772f 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -836,6 +836,7 @@ static struct option const long_options[] =
   {block-size, required_argument, NULL, BLOCK_SIZE_OPTION},
   {context, no_argument, 0, 'Z'},
   {author, no_argument, NULL, AUTHOR_OPTION},
+  {digit-group, no_argument, NULL, 'y'},
   {GETOPT_HELP_OPTION_DECL},
   {GETOPT_VERSION_OPTION_DECL},
   {NULL, 0, NULL, 0}
@@ -1639,7 +1640,7 @@ decode_switches (int argc, char **argv)
 {
   int oi = -1;
   int c = getopt_long (argc, argv,
-   abcdfghiklmnopqrstuvw:xABCDFGHI:LNQRST:UXZ1,
+   abcdfghiklmnopqrstuvw:xyABCDFGHI:LNQRST:UXZ1,
long_options, oi);
   if (c == -1)
 break;
@@ -1760,6 +1761,10 @@ decode_switches (int argc, char **argv)
   format = horizontal;
   break;
 
+case 'y':
+  human_output_opts = human_group_digits;
+  break;
+
 case 'A':
   if (ignore_mode == IGNORE_DEFAULT)
 ignore_mode = IGNORE_DOT_AND_DOTDOT;
@@ -4708,6 +4713,8 @@ Mandatory arguments to long options are mandatory for 
short options too.\n\
   -w, --width=COLS   assume screen width instead of current value\n\
   -x list entries by lines instead of by columns\n\
   -X sort alphabetically by entry extension\n\
+  -y, --digit-group  with -l, group digits of file size\n\
+
   -Z, --context  print any SELinux security context of each file\n\
   -1 list one file per line\n\
 ), stdout);

bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[jeff.liu]

Jim Meyering wrote:
 Paul Eggert wrote:
 Jeff Liu and Jim Meyering wrote:
 diff --git a/src/fiemap.h b/src/fiemap.h
 new file mode 100644
 index 000..d33293b
 --- /dev/null
 +++ b/src/fiemap.h
 
 Hi Paul,
 
 Thanks for the review.
 
 Why is this file necessary?  fiemap.h is included only if it exists,
 right?  Shouldn't we just use the kernel's fiemap.h rather than
 copying it here and assuming kernel internals?
 
 The ioctl is available/usable in 2.6.27 and newer that do not publish
 this file.  For example, it's in F13's (2.6.33's) /usr/include/linux/fiemap.h,
 as well as the one in debian unstable's 2.6.32, but probably
 not in much older kernels.
 
 Hmm..  I see that it's available even in F11's 2.6.30.9-x
 
 It would be better to include linux/fiemap.h when present,
 else our copy of that header.  Then, eventually, the else
 clause will become unused.  Note that even on newer kernels,
 the linux/* headers are optional.
 
 Eventually we'll have a hard requirement on kernel headers --
 at least when building against a linux kernel.
 
  if (lseek (src_fd, ext_logical, SEEK_SET)  0LL)
 For this sort of thing, please just use 0 rather than 0LL.
 0 is easier to read and it has the same effect here.
 
 Included in the patch below.
 
  char buf[buf_size];
 This assumes C99, since buf_size is not known at compile time.
 Also, isn't there a potential segmentation-violation problem
 if buf_size is sufficiently large?

 More generally, since the caller is already allocating a buffer of the
 appropiate size, shouldn't we just reuse that buffer, rather than
 allocating a new one?  That would avoid the problems of assuming
 C99 and possible segmentation violations.
 
 Good point.  Thanks.
 We can definitely avoid that allocation.
 Do you feel like writing the patch?
 
 I've just pushed this series to a branch, fiemap-copy,
 so others can follow along and contribute more easily.
 
   char fiemap_buf[4096];
   struct fiemap *fiemap = (struct fiemap *) fiemap_buf;
   struct fiemap_extent *fm_ext = fiemap-fm_extents[0];
   uint32_t count = ((sizeof fiemap_buf - sizeof (*fiemap))
 / sizeof (struct fiemap_extent));
 This code isn't portable, since fiemap_buf is only char-aligned, and
 struct fiemap may well require stricter alignment.  The code will work
 on the x86 despite the alignment problem, but that's just a happy
 coincidence.

 A lesser point: the code assumes that 'struct fiemap' is sufficiently
 small (considerably less than 4096 bytes in size); I expect that this
 is universally true but we might as well check this assumption, since
 it's easy to do so without any runtime overhead.

 So I propose something like this instead:

union { struct fiemap f; char c[4096]; } fiemap_buf;
struct fiemap *fiemap = fiemap_buf.f;
struct fiemap_extent *fm_ext = fiemap-fm_extents[0];
enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_ext };
verify (count != 0);
Cl!!

Thanks Paul and Jim for the sharing.


Regards,
-Jeff

 
 I've done this in your name:
 
 From fffa2e8661a27978927fcc8afb6873631a753292 Mon Sep 17 00:00:00 2001
 From: Paul Eggert egg...@cs.ucla.edu
 Date: Wed, 9 Jun 2010 08:15:07 +0200
 Subject: [PATCH] copy.c: ensure proper alignment of fiemap buffer
 
 * src/copy.c (fiemap_copy): Ensure that our fiemap buffer
 is large enough and well-aligned.
 Replace 0LL with equivalent 0 as 3rd argument to lseek.
 ---
  src/copy.c |   15 ---
  1 files changed, 8 insertions(+), 7 deletions(-)
 
 diff --git a/src/copy.c b/src/copy.c
 index f629771..27e083a 100644
 --- a/src/copy.c
 +++ b/src/copy.c
 @@ -171,11 +171,12 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size,
   char const *dst_name, bool *normal_copy_required)
  {
bool last = false;
 -  char fiemap_buf[4096];
 -  struct fiemap *fiemap = (struct fiemap *) fiemap_buf;
 +  union { struct fiemap f; char c[4096]; } fiemap_buf;
 +  struct fiemap *fiemap = fiemap_buf.f;
struct fiemap_extent *fm_ext = fiemap-fm_extents[0];
 -  uint32_t count = ((sizeof fiemap_buf - sizeof (*fiemap))
 -/ sizeof (struct fiemap_extent));
 +  enum { count = (sizeof fiemap_buf - sizeof *fiemap) / sizeof *fm_ext };
 +  verify (count != 0);
 +
off_t last_ext_logical = 0;
uint64_t last_ext_len = 0;
uint64_t last_read_size = 0;
 @@ -185,7 +186,7 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size,
/* This is required at least to initialize fiemap-fm_start,
   but also serves (in mid 2010) to appease valgrind, which
   appears not to know the semantics of the FIEMAP ioctl. */
 -  memset (fiemap_buf, 0, sizeof fiemap_buf);
 +  memset (fiemap_buf, 0, sizeof fiemap_buf);
 
do
  {
 @@ -220,13 +221,13 @@ fiemap_copy (int src_fd, int dest_fd, size_t buf_size,
off_t ext_logical = fm_ext[i].fe_logical;
uint64_t ext_len = fm_ext[i].fe_length;
 
 -  if (lseek (src_fd, ext_logical, SEEK_SET)  0LL)
 +   

bug#6381: [PATCH] Add digit grouping option for file sizes in ls

[Jim Meyering]

Sam Bobroff wrote:
 Please excuse me if I'm not going about this the right way but I would
 like to offer a tiny patch for ls that adds an option to turn on digit
 grouping for file sizes in the long output format.

 (The support for displaying digit grouping is already present, all that
 is required is an option for turning it on.)

 I find it a very nice way to view file sizes because it makes it easy to
 read the size of large files (like -h) but it does not hide any
 information (unlike -h).

 For example:

 $ ls -l bigfile
 -rw-r--r-- 1 samb samb 2146034154 2010-06-08 14:55 big.log

 $ ls -ly bigfile
 -rw-r--r-- 1 samb samb 2,146,034,154 2010-06-08 14:55 big.log

Thanks, but you can do that already.
Use --block-size='1 and be sure you're using a locale
that uses the thousands-grouping byte you're used to:

$ dd if=/dev/zero of=big bs=1k count=1 seek=1G
...
$ LC_ALL=en_US ls -lgo --block-size='1 big
-rw---. 1 1,099,511,628,800 2010-06-09 09:32 big

bug#6366: join can't join on numeric fields

[Alex Shinn]

On Wed, Jun 9, 2010 at 3:56 PM, Jim Meyering j...@meyering.net wrote:

 When comparing floating point numbers how would join measure equality?
 Should it consider 1.001e2 to be equal to 100.0 ?
 What if the maximum precision available does not
 allow us to distinguish those two values?

Indeed, that's why you generally don't use floating
point numbers as database keys.

We could either restrict the numbers to integers,
or continue to allow floats with a note to the effect
that float precision is machine-specific.  Personally
I have no need for floats so am happy with the former.

-- 
Alex

bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[jeff.liu]

Jim Meyering wrote:
 Jim Meyering wrote:
 Subject: [PATCH 01/10] cp: Add FIEMAP support for efficient sparse file copy
 
 FYI, using those patches, I ran a test for the first time in a few days:
 
 check -C tests TESTS=cp/sparse-fiemap VERBOSE=yes
 
 It failed like this on an ext4 partition using F13:
 
 + timeout 10 cp --sparse=always sparse fiemap
 + fail=1
 ++ stat --printf %s sparse
 ++ stat --printf %s fiemap
 + test 1099511628800 = 0
 + fail=1
 
 That is very odd.  No diagnostic from cp, yet it failed
 after creating a zero-length file.
 
 Here's the corresponding piece of the script:
 
 # It takes many minutes to copy this sparse file using the old method.
 # By contrast, it takes far less than 1 second using FIEMAP-copy.
 timeout 10 cp --sparse=always sparse fiemap || fail=1
 
 # Ensure that the sparse file copied through fiemap has the same size
 # in bytes as the original.
 test $(stat --printf %s sparse) = $(stat --printf %s fiemap) || fail=1
 
 However, so far I've been unable to reproduce the failure,
 running hundreds of iterations:
 
 for i in $(seq 300); do printf .; make check -C tests \
   TESTS=cp/sparse-fiemap VERBOSE=yes  makerr-$i || break; done
 
 Have any of you heard of a problem whereby a cold cache can cause
 such a thing?  echo 3  /proc/sys/vm/drop_caches didn't help.
Hi Jim,

Have you run `sync' before clean the buffer and caches?  Actually, even run 
`sync' first, sometimes,
maybe the dirty objects still can not be freed in some cases. :(

I can reproduce this issue on ext4 and btrfs(physical mounted partition) or 
just run the
sparse-fiemap test script, ocfs2 always works fine in this case.

I guess this issue might caused by the 'cold cache' as your above mentioned.
According to my tryout, after clean out the caches, cp via filemap always works 
in my test
environment, otherwise, it failed from time to time.

My kernel version:
Linux jeff-laptop 2.6.33-rc5-00238-gb04da8b-dirty #11 SMP Sat Dec 19 22:02:01 
CST 2009 i686 GNU/Linux

j...@jeff-laptop:/ext4$ dd if=/dev/zero of=sparse bs=1k count=1 seek=1G
1+0 records in
1+0 records out
1024 bytes (1.0 kB) copied, 0.000156654 s, 6.5 MB/s
j...@jeff-laptop:/ext4$ ls -l sparse
-rw-r--r-- 1 jeff jeff 1099511628800 Jun  9 22:21 sparse
j...@jeff-laptop:/ext4$ filefrag sparse
sparse: 0 extents found
j...@jeff-laptop:/ext4$ filefrag -v sparse
Filesystem type is: ef53
File size of sparse is 1099511628800 (268435457 blocks, blocksize 4096)
 ext  logical physical expected length flags
sparse: 1 extent found

To free the buffer cache:
=
j...@jeff-laptop:/ext4$ free
 total   used   free sharedbuffers cached
Mem:   1980300 7199721260328  0   2836  94104
-/+ buffers/cache: 6230321357268
Swap:0  0  0
j...@jeff-laptop:/ext4$ sync

In another root console, run 'echo 3  /proc/sys/vm/drop_caches'

j...@jeff-laptop:/ext4$ free
 total   used   free sharedbuffers cached
Mem:   1980300 7167801263520  0   1184  88592   
-freed
-/+ buffers/cache: 6270041353296
Swap:0  0  0

j...@jeff-laptop:/ext4$ filefrag -v sparse
Filesystem type is: ef53
File size of sparse is 1099511628800 (268435457 blocks, blocksize 4096)
 ext  logical physical expected length flags
   0 26843545632999   1 eof
sparse: 2 extents found

j...@jeff-laptop:/ext4$ ./cp --sparse=always sparse f1
last_ext_logical 1099511627776 last_read_size 1024 src_total_size 1099511628800
j...@jeff-laptop:/ext4$ filefrag -v f1
Filesystem type is: ef53
File size of f1 is 1099511628800 (268435457 blocks, blocksize 4096)
 ext  logical physical expected length flags
   0 268435456   296960   1 eof
f1: 2 extents found


j...@jeff-laptop:/ext4$ ./cp --sparse=always sparse f2
last_ext_logical 1099511627776 last_read_size 1024 src_total_size 1099511628800

j...@jeff-laptop:/ext4$ filefrag -v f2
Filesystem type is: ef53
File size of f2 is 1099511628800 (268435457 blocks, blocksize 4096)
 ext  logical physical expected length flags
f2: 1 extent found

j...@jeff-laptop:/ext4$ sync and 'clean memory via /proc on another root 
console'

j...@jeff-laptop:/ext4$ filefrag -v f2
Filesystem type is: ef53
File size of f2 is 1099511628800 (268435457 blocks, blocksize 4096)
 ext  logical physical expected length flags
   0 26843545633379   1 eof
f2: 2 extents found


I will do a double check for my original patch to ensure this is not a code bug 
for that issue once
get through an urgent task on hand.

Thanks,
-Jeff

 I suspect that having so many extents is unusual, so maybe
 this is a rarely exercised corner case.
 
 ===
 As I wrote the above, I realized I probably had enough
 information to deduce where things were going wrong, even
 if so far I've been unable to reproduce it.

bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[Jim Meyering]

jeff.liu wrote:

 Jim Meyering wrote:
 Jim Meyering wrote:
 Subject: [PATCH 01/10] cp: Add FIEMAP support for efficient sparse file copy

 FYI, using those patches, I ran a test for the first time in a few days:

 check -C tests TESTS=cp/sparse-fiemap VERBOSE=yes

 It failed like this on an ext4 partition using F13:

 + timeout 10 cp --sparse=always sparse fiemap
 + fail=1
 ++ stat --printf %s sparse
 ++ stat --printf %s fiemap
 + test 1099511628800 = 0
 + fail=1

 That is very odd.  No diagnostic from cp, yet it failed
 after creating a zero-length file.

 Here's the corresponding piece of the script:

 # It takes many minutes to copy this sparse file using the old method.
 # By contrast, it takes far less than 1 second using FIEMAP-copy.
 timeout 10 cp --sparse=always sparse fiemap || fail=1

 # Ensure that the sparse file copied through fiemap has the same size
 # in bytes as the original.
 test $(stat --printf %s sparse) = $(stat --printf %s fiemap) || fail=1

 However, so far I've been unable to reproduce the failure,
 running hundreds of iterations:

 for i in $(seq 300); do printf .; make check -C tests \
   TESTS=cp/sparse-fiemap VERBOSE=yes  makerr-$i || break; done

 Have any of you heard of a problem whereby a cold cache can cause
 such a thing?  echo 3  /proc/sys/vm/drop_caches didn't help.
 Hi Jim,

 Have you run `sync' before clean the buffer and caches?  Actually, even run 
 `sync' first, sometimes,
 maybe the dirty objects still can not be freed in some cases. :(

Hi Jeff,

Thanks for the feedback.

The test script I ran above does not invoke sync between
the dd invocation and the cp --sparse.
If running sync before cp is required in order to avoid a failure,
then I consider that a bug in cp.

 I can reproduce this issue on ext4 and btrfs(physical mounted partition) or 
 just run the

If you can reproduce the cp failure, then please use my latest
patch that diagnoses 2nd and subsequent FIEMAP ioctl failure
and tell me what cp prints when it fails.

I would really like to know how the first ioctl can succeed,
yet the second one fails.

Your demonstrations show that *after* the cp, one may have
to sync in order to see precisely if/how the newly-created
destination file is fragmented, but that is nothing new.
Note the commented-out sync uses in the test script.
As far as I can see, your examples do not show cp failing
like it did for me.

bug#6383: cat has misleading documentation

[Chas. Owens]

The -n option is documented as number all output lines, but when -b
is present it only numbers non-blank lines.  The documentation should
read number output lines.

-- 
Chas. Owens
wonkden.net
The most important skill a programmer can have is the ability to read.

bug#6383: [PATCH] cat: improve documentation

[Eric Blake]

* src/cat.c (usage): Clarify that -b overrides -n.
* doc/coreutils.texi (cat invocation): Likewise.
* THANKS: Update.
Suggested by Chas. Owens, in bug 6383.
---

 The -n option is documented as number all output lines, but when -b
 is present it only numbers non-blank lines.  The documentation should
 read number output lines.

Thanks for the report.  -b in isolation also numbers lines; so it is
more an issue that -b overrides -n.  How about this patch, instead?

 THANKS |1 +
 doc/coreutils.texi |3 ++-
 src/cat.c  |2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/THANKS b/THANKS
index dce3c94..caa40de 100644
--- a/THANKS
+++ b/THANKS
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 26b4eba..b27d9d7 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -1494,7 +1494,8 @@ cat invocation
 @itemx --number
 @opindex -n
 @opindex --number
-Number all output lines, starting with 1.
+Number all output lines, starting with 1.  This option is ignored
+if @option{-b} is in effect.

 @item -s
 @itemx --squeeze-blank
diff --git a/src/cat.c b/src/cat.c
index eebfb97..c4a2a9e 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -92,7 +92,7 @@ Usage: %s [OPTION]... [FILE]...\n\
 Concatenate FILE(s), or standard input, to standard output.\n\
 \n\
   -A, --show-all   equivalent to -vET\n\
-  -b, --number-nonblanknumber nonempty output lines\n\
+  -b, --number-nonblanknumber nonempty output lines, overrides -n\n\
   -e   equivalent to -vE\n\
   -E, --show-ends  display $ at end of each line\n\
   -n, --number number all output lines\n\
-- 
1.7.0.1

bug#6383: [PATCH] cat: improve documentation

[Chas. Owens]

That would work as well.  I suggested removing the all because it
was the simplest solution and would move the text closer to what BSD
uses, but this spells out the behavior explicitly.

On Wed, Jun 9, 2010 at 13:17, Eric Blake ebl...@redhat.com wrote:
 * src/cat.c (usage): Clarify that -b overrides -n.
 * doc/coreutils.texi (cat invocation): Likewise.
 * THANKS: Update.
 Suggested by Chas. Owens, in bug 6383.
 ---

 The -n option is documented as number all output lines, but when -b
 is present it only numbers non-blank lines.  The documentation should
 read number output lines.

 Thanks for the report.  -b in isolation also numbers lines; so it is
 more an issue that -b overrides -n.  How about this patch, instead?

  THANKS             |    1 +
  doc/coreutils.texi |    3 ++-
  src/cat.c          |    2 +-
  3 files changed, 4 insertions(+), 2 deletions(-)

 diff --git a/THANKS b/THANKS
 index dce3c94..caa40de 100644
 --- a/THANKS
 +++ b/THANKS
 diff --git a/doc/coreutils.texi b/doc/coreutils.texi
 index 26b4eba..b27d9d7 100644
 --- a/doc/coreutils.texi
 +++ b/doc/coreutils.texi
 @@ -1494,7 +1494,8 @@ cat invocation
 �...@itemx --number
 �...@opindex -n
 �...@opindex --number
 -Number all output lines, starting with 1.
 +Number all output lines, starting with 1.  This option is ignored
 +if @option{-b} is in effect.

 �...@item -s
 �...@itemx --squeeze-blank
 diff --git a/src/cat.c b/src/cat.c
 index eebfb97..c4a2a9e 100644
 --- a/src/cat.c
 +++ b/src/cat.c
 @@ -92,7 +92,7 @@ Usage: %s [OPTION]... [FILE]...\n\
  Concatenate FILE(s), or standard input, to standard output.\n\
  \n\
   -A, --show-all           equivalent to -vET\n\
 -  -b, --number-nonblank    number nonempty output lines\n\
 +  -b, --number-nonblank    number nonempty output lines, overrides -n\n\
   -e                       equivalent to -vE\n\
   -E, --show-ends          display $ at end of each line\n\
   -n, --number             number all output lines\n\
 --
 1.7.0.1





-- 
Chas. Owens
wonkden.net
The most important skill a programmer can have is the ability to read.