Re: Problems with sum in textutils

[Jim Meyering]

Hi Paul,

Thanks for digging up that documentation and for the patch.
I've applied it and added a test case.

Paul Eggert <address@hidden> wrote:
>> From: Jim Meyering <address@hidden>
>> Date: Sun, 28 Oct 2001 01:40:29 +0200
>
>>   [on Solaris 5.8]
>>   $ perl -e 'print chr(255) x 16843010'|/bin/sum
>>   254 32897
>>
>> I think that first number should be 255.
>
> No, 254 is correct.  The problem is that the GNU code is reducing at
> the wrong place; it should reduce at the end, not after every input byte.
> The NetBSD documentation makes this pretty clear; e.g. see
> <http://www.tac.eu.org/cgi-bin/man-cgi?sum+1>
>
> Here is a patch, which cleans up a few other minor glitches
> that I noticed in the neighborhood (sorry, I couldn't resist :-).
>
> 2001-10-27  Paul Eggert  <address@hidden>
>
>       * src/sum.c: Include human.h.
>
>       (ROTATE_RIGHT): Remove; it was slow and the comment was
>       misleading anyway.
>
>       (bsd_sum_file): Avoid unsigned and long when they're not needed.
>       Compute total input bytes as uintmax_t, not long, so that the
>       code works even with files whose size does not fit in 'long'.
>       (sysv_sum_file): Likewise.
>
>       (sysv_sum_file): Do not reduce checksum until the end; this is
>       what System V does.
>
> ===================================================================
> RCS file: src/sum.c,v
> retrieving revision 2.0.16.0
> retrieving revision 2.0.16.1
> diff -pu -r2.0.16.0 -r2.0.16.1
> --- src/sum.c 2001/08/13 10:29:59     2.0.16.0
> +++ src/sum.c 2001/10/28 06:21:15     2.0.16.1
> @@ -27,6 +27,7 @@
>  #include "system.h"
>  #include "closeout.h"
>  #include "error.h"
> +#include "human.h"
>  #include "safe-read.h"
>  
>  /* The official name of this program (e.g., no `g' prefix).  */
> @@ -40,9 +41,6 @@ char *program_name;
>  /* Nonzero if any of the files read were the standard input. */
>  static int have_read_stdin;
>  
> -/* Right-rotate 32-bit integer variable C. */
> -#define ROTATE_RIGHT(c) if ((c) & 01) (c) = ((c) >>1) + 0x8000; else (c) >>= 
> 1;
> -
>  static struct option const longopts[] =
>  {
>    {"sysv", no_argument, NULL, 's'},
> @@ -88,9 +86,10 @@ static int
>  bsd_sum_file (const char *file, int print_name)
>  {
>    register FILE *fp;
> -  register unsigned long checksum = 0; /* The checksum mod 2^16. */
> -  register long total_bytes = 0; /* The number of bytes. */
> +  register int checksum = 0; /* The checksum mod 2^16. */
> +  register uintmax_t total_bytes = 0;        /* The number of bytes. */
>    register int ch;           /* Each character read. */
> +  char hbuf[LONGEST_HUMAN_READABLE + 1];
>  
>    if (STREQ (file, "-"))
>      {
> @@ -112,7 +111,7 @@ bsd_sum_file (const char *file, int prin
>    while ((ch = getc (fp)) != EOF)
>      {
>        total_bytes++;
> -      ROTATE_RIGHT (checksum);
> +      checksum = (checksum >> 1) + ((checksum & 1) << 15);
>        checksum += ch;
>        checksum &= 0xffff;    /* Keep it within bounds. */
>      }
> @@ -131,7 +130,8 @@ bsd_sum_file (const char *file, int prin
>        return -1;
>      }
>  
> -  printf ("%05lu %5ld", checksum, (total_bytes + 1024 - 1) / 1024);
> +  printf ("%05d %5s", checksum,
> +       human_readable_inexact (total_bytes, hbuf, 1, 1024, human_ceiling));
>    if (print_name > 1)
>      printf (" %s", file);
>    putchar ('\n');
> @@ -150,8 +150,13 @@ sysv_sum_file (const char *file, int pri
>    int fd;
>    unsigned char buf[8192];
>    register int bytes_read;
> -  register unsigned long checksum = 0;
> -  long total_bytes = 0;
> +  uintmax_t total_bytes = 0;
> +  char hbuf[LONGEST_HUMAN_READABLE + 1];
> +  int r;
> +  int checksum;
> +
> +  /* The sum of all the input bytes, modulo (UINT_MAX + 1).  */
> +  register unsigned int s = 0;
>  
>    if (STREQ (file, "-"))
>      {
> @@ -175,10 +180,7 @@ sysv_sum_file (const char *file, int pri
>        register int i;
>  
>        for (i = 0; i < bytes_read; i++)
> -     checksum += buf[i];
> -      /* Reduce checksum mod 0xffff, to avoid overflow.  */
> -      checksum = (checksum & 0xffff) + (checksum >> 16);
> -
> +     s += buf[i];
>        total_bytes += bytes_read;
>      }
>  
> @@ -196,7 +198,11 @@ sysv_sum_file (const char *file, int pri
>        return -1;
>      }
>  
> -  printf ("%lu %ld", checksum % 0xffff, (total_bytes + 512 - 1) / 512);
> +  r = (s & 0xffff) + ((s & 0xffffffff) >> 16);
> +  checksum = (r & 0xffff) + (r >> 16);
> +
> +  printf ("%d %s", checksum,
> +       human_readable_inexact (total_bytes, hbuf, 1, 512, human_ceiling));
>    if (print_name)
>      printf (" %s", file);
>    putchar ('\n');