Re: sharutils-4.3.61 (test release)

[Stepan Kasal]

Hallo,
   I looked at the uu{de,en}code.c files in

> > http://www.iro.umontreal.ca/~eichwalk/sharutils/sharutils-4.3.61.tar.bz2

the code for encode() seems a bit weird to me.

For example,

|      do
|       {
|         register int m = fread (buf, 1, 45 - n, stdin);
|         if (m == 0)
|           break;
|         n += m;
|       }
|      while (n < 45);

is wrong, there should be &buf[n] instead of buf.
But it seems that fread doesn't perform partial reads if there is neither error
nor eof.  I've cleaned up the code, I hope.

I've also decided to use a new function, rw_error(), to report errors.
I guess it's better if the gettextised "Write error" appears only once,
is it true?

I also check for error after every putchar(), is this right?
(I imagined that someone decided to send the result to a floppy which
doesn't work.  Wouldn't each of the repeated putchar() calls require
new timeout on some platforms?  If yes, my code shortens the time until
the error is finally reported.)

As far as uudecode is concerned, I've only added fflush(stdout) and checked for
error, which is the flaw reported by Paul yesterday.  (Though fclose(stdin) may
fail, checking for it is not as important as flushing the write buffer
properly.)

putchar() calls in uudecode are left as they are, until someone explains me
whether the try_putchar() idea in uuencode is right.

The patch (enclosed below) is relative to plain sharutils-4.3.61, without the
yesterday's patch by Paul Eggert.

Looking forward to your feedback.
                                Stepan Kasal


2002-07-03  Stepan Kasal  <address@hidden>

        * src/uuencode.c (rw_error): new function
        (try_putchar): new function
        (encode): code reorganised, check for errors
        (main): more checks for errros
        * src/uudecode.c (decode): call fflush at the end and check for errors

diff -urN sharutils-4.3.61.orig/src/uudecode.c sharutils-4.3.61/src/uudecode.c
--- sharutils-4.3.61.orig/src/uudecode.c        Mon Jul  1 17:34:52 2002
+++ sharutils-4.3.61/src/uudecode.c     Wed Jul  3 16:49:47 2002
@@ -279,6 +279,7 @@
   char buf[2 * BUFSIZ];
   char *outname;
   int do_base64 = 0;
+  int rval;
 
   /* Search for header line.  */
 
@@ -357,9 +358,17 @@
 
   /* For each input line:  */
   if (do_base64)
-    return read_base64 (inname);
+    rval = read_base64 (inname);
   else
-    return read_stduu (inname);
+    rval = read_stduu (inname);
+
+  if (ferror(stdout) || fflush(stdout) != 0)
+    {
+      error (0, 0, _("%s: write error"), outname);
+      return 1;
+    }
+
+  return rval;
 }
 
 static void
@@ -419,7 +428,7 @@
 This is free software; see the source for copying conditions.  There is NO\n\
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\
 "),
-                 "1994, 1995, 1996");
+                 "1994, 1995, 1996, 2002");
          exit (EXIT_SUCCESS);
 
        case 0:
diff -urN sharutils-4.3.61.orig/src/uuencode.c sharutils-4.3.61/src/uuencode.c
--- sharutils-4.3.61.orig/src/uuencode.c        Thu Jun 27 20:57:55 2002
+++ sharutils-4.3.61/src/uuencode.c     Wed Jul  3 16:32:46 2002
@@ -69,6 +69,8 @@
   { NULL, 0, 0, 0 }
 };
 
+static void rw_error __P ((int,int));
+static inline void try_putchar __P ((int));
 static void encode __P ((void));
 static void usage __P ((int));
 
@@ -107,6 +109,25 @@
 /* ENC is the basic 1 character encoding function to make a char printing.  */
 #define ENC(Char) (trans_ptr[(Char) & 077])
 
+#define RWE_READ  0
+#define RWE_WRITE 1
+
+static void
+rw_error (rw_errno, rw_type)
+         int rw_errno, rw_type;
+{
+  error (EXIT_FAILURE, rw_errno,
+         rw_type == RWE_WRITE ? _("Write error") : _("Read error"));
+}
+
+static inline void
+try_putchar (c)
+         int c;
+{
+  if (putchar (c) == EOF)
+    rw_error (0, RWE_WRITE);
+}
+
 /*------------------------------------------------.
 | Copy from IN to OUT, encoding as you go along.  |
 `------------------------------------------------*/
@@ -115,91 +136,56 @@
 encode ()
 {
   register int ch, n;
+  int finishing = 0;
   register char *p;
-  char buf[80];
+  char buf[46];  /* 45 should be enough, but one never knows... */
 
-  while (1)
+  while ( !finishing && (n = fread (buf, 1, 45, stdin)) > 0 )
     {
-      n = 0;
-      do
-       {
-         register int m = fread (buf, 1, 45 - n, stdin);
-         if (m == 0)
-           break;
-         n += m;
-       }
-      while (n < 45);
-
-      if (n == 0)
-       break;
+      if (n < 45)
+        {
+          if (feof (stdin))
+            finishing = 1;
+          else
+            rw_error (0, RWE_READ);
+        }
 
       if (trans_ptr == uu_std)
-       if (putchar (ENC (n)) == EOF)
-         break;
-      for (p = buf; n > 2; n -= 3, p += 3)
-       {
-         ch = *p >> 2;
-         ch = ENC (ch);
-         if (putchar (ch) == EOF)
-           break;
-         ch = ((*p << 4) & 060) | ((p[1] >> 4) & 017);
-         ch = ENC (ch);
-         if (putchar (ch) == EOF)
-           break;
-         ch = ((p[1] << 2) & 074) | ((p[2] >> 6) & 03);
-         ch = ENC (ch);
-         if (putchar (ch) == EOF)
-           break;
-         ch = p[2] & 077;
-         ch = ENC (ch);
-         if (putchar (ch) == EOF)
-           break;
-       }
+        putchar (ENC (n));
 
-      if (n != 0)
-       break;
+      for (p = buf; n > 2; n -= 3, p += 3)
+        {
+          try_putchar (ENC (*p >> 2));
+          try_putchar (ENC ((*p << 4) & 060) | ((p[1] >> 4) & 017));
+          try_putchar (ENC ((p[1] << 2) & 074) | ((p[2] >> 6) & 03));
+          try_putchar (ENC (p[2] & 077));
+        }
+
+      if (n > 0)  /* encode the last one or two chars */
+        {
+          char tail = trans_ptr == uu_std ? ENC ('\0') : '=';
+
+          if (n == 1)
+            p[1] = '\0';
+
+          try_putchar (ENC (*p >> 2));
+          try_putchar (ENC ((*p << 4) & 060) | ((p[1] >> 4) & 017));
+          try_putchar (n == 1 ? tail : ENC ((p[1] << 2) & 074));
+          try_putchar (tail);
+        }
 
-      if (putchar ('\n') == EOF)
-       break;
-    }
-
-  while (n != 0)
-    {
-      char c1 = *p;
-      char c2 = n == 1 ? 0 : p[1];
-
-      ch = c1 >> 2;
-      ch = ENC (ch);
-      if (putchar (ch) == EOF)
-       break;
-
-      ch = ((c1 << 4) & 060) | ((c2 >> 4) & 017);
-      ch = ENC (ch);
-      if (putchar (ch) == EOF)
-       break;
-
-      if (n == 1)
-       ch = trans_ptr == uu_std ? ENC ('\0') : '=';
-      else
-       {
-         ch = (c2 << 2) & 074;
-         ch = ENC (ch);
-       }
-      if (putchar (ch) == EOF)
-       break;
-      ch = trans_ptr == uu_std ? ENC ('\0') : '=';
-      if (putchar (ch) == EOF)
-       break;
-      putchar ('\n');
-      break;
+      try_putchar ('\n');
     }
 
   if (ferror (stdin))
-    error (EXIT_FAILURE, 0, _("Read error"));
+    rw_error (0, RWE_READ);
+  if (fclose (stdin) != 0)
+    rw_error (errno, RWE_READ);
+
   if (trans_ptr == uu_std)
     {
-      putchar (ENC ('\0'));
-      putchar ('\n');
+      try_putchar (ENC ('\0'));
+      try_putchar ('\n');
     }
 }
 
@@ -261,7 +247,7 @@
 This is free software; see the source for copying conditions.  There is NO\n\
 warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.\n\
 "),
-                 "1994, 1995, 1996");
+                 "1994, 1995, 1996, 2002");
          exit (EXIT_SUCCESS);
 
        case 0:
@@ -297,11 +283,16 @@
 choke me - Must translate mode argument
 #endif
 
-  printf ("begin%s %o %s\n", trans_ptr == uu_std ? "" : "-base64",
-         mode, argv[optind]);
+  if (printf ("begin%s %o %s\n", trans_ptr == uu_std ? "" : "-base64",
+             mode, argv[optind]) < 0)
+    rw_error (0, RWE_WRITE);
+
   encode ();
-  printf (trans_ptr == uu_std ? "end\n" : "====\n");
-  if (ferror (stdout))
-    error (EXIT_FAILURE, 0, _("Write error"));
+
+  if (ferror (stdout) ||
+      printf (trans_ptr == uu_std ? "end\n" : "====\n") < 0 ||
+      fclose (stdout) != 0)
+    rw_error (0, RWE_WRITE);
+
   exit (EXIT_SUCCESS);
 }