cleanup

git-svn-id: http://svn.code.sf.net/p/netpbm/code/trunk@4759 9d0c8265-081b-0410-96cb-a4ca84ce46f8

4 files changed, 239 insertions, 179 deletions

diff --git a/urt/rle.h b/urt/rle.h
index b3263049..ee9eb379 100644
--- a/urt/rle.h
+++ b/urt/rle.h
@@ -327,9 +327,9 @@ rle_open_f_noexit(const char * const prog_name,
 
 /* Append history information to the HISTORY comment. */
 void
-rle_addhist(char *          argv[],
-            rle_hdr * const in_hdr,
-            rle_hdr * const out_hdr);
+rle_addhist(const char ** const argv,
+            rle_hdr *     const in_hdr,
+            rle_hdr *     const out_hdr);
 
 /* From cmd_name.c. */
 /*****************************************************************
diff --git a/urt/rle_addhist.c b/urt/rle_addhist.c
index 9debec28..f319a40e 100644
--- a/urt/rle_addhist.c
+++ b/urt/rle_addhist.c
@@ -30,82 +30,118 @@
 #include <time.h>
 
 #include "netpbm/mallocvar.h"
+
 #include "rle.h"
 
 
-/*****************************************************************
- * TAG( rle_addhist )
- *
- * Put a history comment into the header struct.
- * Inputs:
- *  argv:       Command line history to add to comments.
- *  in_hdr:     Incoming header struct to use.
- * Outputs:
- *  out_hdr:    Outgoing header struct to add to.
- * Assumptions:
- *  If no incoming struct then value is NULL.
- * Algorithm:
- *  Calculate length of all comment strings, malloc and then set via
- *  rle_putcom.
- */
 
-void
-rle_addhist(char *          argv[],
-            rle_hdr * const in_hdr,
-            rle_hdr * const out_hdr) {
+static unsigned int
+newCommentLen(const char *  const histoire,
+              const char *  const old,
+              const char ** const argv,
+              const char *  const timedate,
+              const char *  const padding) {
 
-    const char * const histoire = "HISTORY";
-    const char * const padding = "\t";
+    unsigned int length;
+    unsigned int i;
 
-    int length;
-    int i;
-    time_t  temp;
-    /* padding must give number of characters in histoire   */
-    /*     plus one for "="                 */
-    char * timedate;
-    const char * old;
-    static char * newc;
+    length = 0;  /* initial value */
 
-    if (getenv("NO_ADD_RLE_HISTORY"))
-        return;
+    /* Add length of each arg plus space. */
 
-    length = 0;
-    for (i = 0; argv[i]; ++i)
-        length += strlen(argv[i]) +1;   /* length of each arg plus space. */
+    for (i = 0; argv[i]; ++i) {
+        size_t const thisArgLen = strlen(argv[i]);
+        if (thisArgLen < UINT_MAX - length - 100) {
+            length += thisArgLen;
+            length += 1;  /* For the space */
+        }
+    }
 
-    time(&temp);
-    timedate = ctime(&temp);
-    length += strlen(timedate);        /* length of date and time in ASCII. */
+    /* Add length of date and time in ASCII. */
+    length += strlen(timedate);
 
+    /* Add length of padding, "on ", and length of history name plus "="*/
     length += strlen(padding) + 3 + strlen(histoire) + 1;
-        /* length of padding, "on "  and length of history name plus "="*/
-    if (in_hdr) /* if we are interested in the old comments... */
-        old = rle_getcom(histoire, in_hdr);     /* get old comment. */
-    else
-        old = NULL;
 
     if (old && *old)
         length += strlen(old);       /* add length if there. */
 
-    ++length;                               /*Cater for the null. */
+    ++length;     /* Add size of terminating NUL. */
 
-    MALLOCARRAY(newc, length);
+    return length;
+}
 
-    if (newc == NULL)
-        return;
 
-    strcpy(newc,histoire);(void)strcat(newc,"=");
-    if (old && *old)
-        strcat(newc, old); /* add old string if there. */
-    for (i=0;argv[i];i++) {
-        strcat(newc, argv[i]);
-        strcat(newc, " ");
-    }
-    strcat(newc,"on ");
-    strcat(newc,timedate);         /* \n supplied by time. */
-    strcat(newc,padding);          /* to line up multiple histories.*/
 
-    rle_putcom(newc, out_hdr);
+void
+rle_addhist(const char ** const argv,
+            rle_hdr *     const inHdrP,
+            rle_hdr *     const outHdrP) {
+/*----------------------------------------------------------------------------
+  Put a history comment into the header struct.
+  Inputs:
+   argv:        Command line history to add to comments.
+   *inHdrP:     Incoming header struct to use.
+  Outputs:
+   *outHdrP:    Outgoing header struct to add to.
+  Assumptions:
+   If no incoming struct then value is NULL.
+  Algorithm:
+   Calculate length of all comment strings, malloc and then set via
+   rle_putcom.
+  If we run out of memory, don't put the history comment in.
+-----------------------------------------------------------------------------*/
+    if (!getenv("NO_ADD_RLE_HISTORY")) {
+        const char * const histoire = "HISTORY";
+        const char * const padding = "\t";
+
+        unsigned int length;
+            /* length of combined comment - the history comment we are adding
+               and any history comment that is already there (to which we
+               append)
+            */
+        time_t  nowTime;
+        /* padding must give number of characters in histoire   */
+        /*     plus one for "="                 */
+        const char * timedate;
+        const char * old;
+        char * newc;
+
+        if (inHdrP) /* if we are interested in the old comments... */
+            old = rle_getcom(histoire, inHdrP);     /* get old comment. */
+        else
+            old = NULL;
+
+        time(&nowTime);
+        timedate = ctime(&nowTime);
+
+        length = newCommentLen(histoire, old, argv, timedate, padding);
+
+        MALLOCARRAY(newc, length);
+
+        if (newc) {
+            unsigned int i;
+
+            strcpy(newc, histoire);
+            strcat(newc, "=");
+
+            if (old)
+                strcat(newc, old); /* add old string if there. */
+
+            for (i = 0; argv[i]; ++i) {
+                strcat(newc, argv[i]);
+                strcat(newc, " ");
+            }
+            strcat(newc, "on ");
+            strcat(newc, timedate);         /* \n supplied by 'ctime'. */
+            strcat(newc, padding);          /* to line up multiple histories.*/
+
+            rle_putcom(newc, outHdrP);
+                /* Creates reference to 'newc', may destroy reference to
+                   previous comment memory, which will thereby leak.
+                */
+        }
+    }
 }
 
 
diff --git a/urt/rle_getrow.c b/urt/rle_getrow.c
index 5d71cda7..6a25c987 100644
--- a/urt/rle_getrow.c
+++ b/urt/rle_getrow.c
@@ -52,6 +52,66 @@
 
 static int     debug_f;     /* If non-zero, print debug info. */
 
+
+
+static void
+readComments(rle_hdr * const hdrP) {
+
+    FILE * ifP = hdrP->rle_file;
+
+    /* There are comments */
+    short comlen;
+    char * cp;
+
+    VAXSHORT(comlen, ifP); /* get comment length */
+
+    if (comlen < 0)
+        pm_error("Negative comment length in RLE header");
+    else if (comlen > 0) {
+        unsigned int const evenlen = (comlen + 1) & ~1;    /* make it even */
+
+        char * commentHeap;
+        unsigned int i;
+
+        MALLOCARRAY(commentHeap, evenlen);
+
+        if (commentHeap == NULL) {
+            pm_error("Malloc failed for comment buffer of size %u "
+                     "in rle_get_setup, reading '%s'",
+                     evenlen, hdrP->file_name );
+        }
+        fread(commentHeap, 1, evenlen, ifP);
+        /* Count the comments */
+        for (i = 0, cp = commentHeap; cp < commentHeap + comlen; ++cp) {
+            if (*cp == '\0')
+                ++i;
+        }
+        ++i;            /* extra for NULL pointer at end */
+        /* Get space to put pointers to comments */
+        MALLOCARRAY(hdrP->comments, i);
+        if (hdrP->comments == NULL) {
+            pm_error("Malloc failed for %d comment pointers "
+                     "in rle_get_setup, reading '%s'",
+                     i, hdrP->file_name );
+        }
+        /* Set comment heap */
+        hdrP->comments[0] = commentHeap;
+
+        /* Set pointers to individual comments in the heap as
+          hdrP->comments[1], hdrP->comments[2], etc.
+        */
+        for (i = 1, cp = commentHeap + 1;
+             cp < commentHeap + comlen;
+             ++cp)
+            if (*(cp - 1) == '\0')
+                hdrP->comments[i++] = cp;
+        hdrP->comments[i] = NULL;
+    } else
+        hdrP->comments = NULL;
+}
+
+
+
 int
 rle_get_setup(rle_hdr * const hdrP) {
 /*-----------------------------------------------------------------------------
@@ -67,16 +127,15 @@ rle_get_setup(rle_hdr * const hdrP) {
      -3 if an immediate EOF is hit (empty input file)
      -4 if an EOF is encountered reading the setup information.
   Assumptions:
-    infile points to the "magic" number in an RLE file (usually  byte 0
-    in the file).
+    input file is positioned to the "magic" number in an RLE file (usually
+    first byte of the file).
   Algorithm:
     Read in the setup info and fill in *hdrP.
 ---------------------------------------------------------------------------- */
     struct XtndRsetup setup;
     short magic;
-    FILE * infile = hdrP->rle_file;
+    FILE * ifP = hdrP->rle_file;
     int i;
-    char * comment_buf;
 
     /* Clear old stuff out of the header. */
     rle_hdr_clear(hdrP);
@@ -84,13 +143,13 @@ rle_get_setup(rle_hdr * const hdrP) {
         rle_names(hdrP, "Urt", "some file", 0);
     ++hdrP->img_num;     /* Count images. */
 
-    VAXSHORT(magic, infile);
-    if (feof(infile))
+    VAXSHORT(magic, ifP);
+    if (feof(ifP))
         return RLE_EMPTY;
     if (magic != RLE_MAGIC)
         return RLE_NOT_RLE;
-    fread(&setup, 1, SETUPSIZE, infile);  /* assume VAX packing */
-    if (feof( infile))
+    fread(&setup, 1, SETUPSIZE, ifP);  /* assume VAX packing */
+    if (feof( ifP))
         return RLE_EOF;
 
     /* Extract information from setup */
@@ -109,12 +168,12 @@ rle_get_setup(rle_hdr * const hdrP) {
         if (!bg_color)
             pm_error("Failed to allocation array for %u background colors",
                      1+(setup.h_ncolors / 2) * 2);
-        fread((char *)bg_color, 1, 1 + (setup.h_ncolors / 2) * 2, infile);
+        fread((char *)bg_color, 1, 1 + (setup.h_ncolors / 2) * 2, ifP);
         for (i = 0; i < setup.h_ncolors; ++i)
             hdrP->bg_color[i] = bg_color[i];
         free(bg_color);
     } else {
-        getc(infile);   /* skip filler byte */
+        getc(ifP);   /* skip filler byte */
         hdrP->bg_color = NULL;
     }
 
@@ -153,63 +212,25 @@ rle_get_setup(rle_hdr * const hdrP) {
                      hdrP->file_name );
             return RLE_NO_SPACE;
         }
-        fread(maptemp, 2, maplen, infile);
+        fread(maptemp, 2, maplen, ifP);
         for (i = 0; i < maplen; ++i)
             hdrP->cmap[i] = vax_gshort(&maptemp[i * 2]);
         free(maptemp);
     }
 
-    /* Check for comments */
-    if (setup.h_flags & H_COMMENT) {
-        short comlen, evenlen;
-        char * cp;
-
-        VAXSHORT(comlen, infile); /* get comment length */
-        evenlen = (comlen + 1) & ~1;    /* make it even */
-        if (evenlen) {
-            MALLOCARRAY(comment_buf, evenlen);
-
-            if (comment_buf == NULL) {
-                pm_error("Malloc failed for comment buffer of size %d "
-                         "in rle_get_setup, reading '%s'",
-                         comlen, hdrP->file_name );
-                return RLE_NO_SPACE;
-            }
-            fread(comment_buf, 1, evenlen, infile);
-            /* Count the comments */
-            for (i = 0, cp = comment_buf; cp < comment_buf + comlen; ++cp)
-                if (*cp == '\0')
-                    ++i;
-            ++i;            /* extra for NULL pointer at end */
-            /* Get space to put pointers to comments */
-            MALLOCARRAY(hdrP->comments, i);
-            if (hdrP->comments == NULL) {
-                pm_error("Malloc failed for %d comment pointers "
-                         "in rle_get_setup, reading '%s'",
-                         i, hdrP->file_name );
-                return RLE_NO_SPACE;
-            }
-            /* Get pointers to the comments */
-            *hdrP->comments = comment_buf;
-            for (i = 1, cp = comment_buf + 1;
-                 cp < comment_buf + comlen;
-                 ++cp)
-                if (*(cp - 1) == '\0')
-                    hdrP->comments[i++] = cp;
-            hdrP->comments[i] = NULL;
-        } else
-            hdrP->comments = NULL;
-    } else
+    if (setup.h_flags & H_COMMENT)
+        readComments(hdrP);
+    else
         hdrP->comments = NULL;
 
     /* Initialize state for rle_getrow */
     hdrP->priv.get.scan_y = hdrP->ymin;
     hdrP->priv.get.vert_skip = 0;
     hdrP->priv.get.is_eof = 0;
-    hdrP->priv.get.is_seek = ftell(infile) > 0;
+    hdrP->priv.get.is_seek = ftell(ifP) > 0;
     debug_f = 0;
 
-    if (!feof(infile))
+    if (!feof(ifP))
         return RLE_SUCCESS; /* success! */
     else {
         hdrP->priv.get.is_eof = 1;
@@ -247,7 +268,7 @@ rle_getrow(rle_hdr *    const hdrP,
    If ymax is reached (or, somehow, passed), continue reading and
    discarding input until end of image.
 ---------------------------------------------------------------------------- */
-    FILE * const infile = hdrP->rle_file;
+    FILE * const ifP = hdrP->rle_file;
 
     rle_pixel * scanc;
 
@@ -308,9 +329,9 @@ rle_getrow(rle_hdr *    const hdrP,
     else
         scanc = NULL;
     for (;;) {
-        inst[0] = getc(infile);
-        inst[1] = getc(infile);
-        if (feof(infile)) {
+        inst[0] = getc(ifP);
+        inst[1] = getc(ifP);
+        if (feof(ifP)) {
             hdrP->priv.get.is_eof = 1;
             break;      /* <--- one of the exits */
         }
@@ -318,7 +339,7 @@ rle_getrow(rle_hdr *    const hdrP,
         switch(OPCODE(inst)) {
         case RSkipLinesOp:
             if (LONGP(inst)) {
-                VAXSHORT(hdrP->priv.get.vert_skip, infile);
+                VAXSHORT(hdrP->priv.get.vert_skip, ifP);
             } else
                 hdrP->priv.get.vert_skip = DATUM(inst);
             if (debug_f)
@@ -343,7 +364,7 @@ rle_getrow(rle_hdr *    const hdrP,
 
         case RSkipPixelsOp:
             if (LONGP(inst)) {
-                VAXSHORT(long_data, infile);
+                VAXSHORT(long_data, ifP);
                 scan_x += long_data;
                 scanc += long_data;
                 if (debug_f)
@@ -358,7 +379,7 @@ rle_getrow(rle_hdr *    const hdrP,
 
         case RByteDataOp:
             if (LONGP(inst)) {
-                VAXSHORT(nc, infile);
+                VAXSHORT(nc, ifP);
             } else
                 nc = DATUM(inst);
             ++nc;
@@ -375,18 +396,18 @@ rle_getrow(rle_hdr *    const hdrP,
                     nc -= ns;
                 } else
                     ns = 0;
-                fread((char *)scanc, 1, nc, infile);
+                fread((char *)scanc, 1, nc, ifP);
                 while (ns-- > 0)
-                    getc(infile);
+                    getc(ifP);
                 if (nc & 0x1)
-                    getc(infile);   /* throw away odd byte */
+                    getc(ifP);   /* throw away odd byte */
             } else {
                 if (hdrP->priv.get.is_seek)
-                    fseek(infile, ((nc + 1) / 2) * 2, 1);
+                    fseek(ifP, ((nc + 1) / 2) * 2, 1);
                 else {
                     int ii;
                     for (ii = ((nc + 1) / 2) * 2; ii > 0; --ii)
-                        getc(infile);  /* discard it */
+                        getc(ifP);  /* discard it */
                 }
             }
             scanc += nc;
@@ -401,13 +422,13 @@ rle_getrow(rle_hdr *    const hdrP,
 
         case RRunDataOp:
             if (LONGP(inst)) {
-                VAXSHORT(nc, infile);
+                VAXSHORT(nc, ifP);
             } else
                 nc = DATUM(inst);
             ++nc;
             scan_x += nc;
 
-            VAXSHORT(word, infile);
+            VAXSHORT(word, ifP);
             if (debug_f)
                 pm_message("Run length %d (to %d), data %02x",
                            nc, scan_x, word);
@@ -453,4 +474,3 @@ rle_getrow(rle_hdr *    const hdrP,
 }
 
 
-
diff --git a/urt/rle_putcom.c b/urt/rle_putcom.c
index 2ecc8d89..ff27cb20 100644
--- a/urt/rle_putcom.c
+++ b/urt/rle_putcom.c
@@ -25,34 +25,35 @@
  * Copyright (c) 1987, University of Utah
  */
 
+#include <limits.h>
+#include <assert.h>
 #include <stdio.h>
 
 #include "netpbm/mallocvar.h"
 #include "netpbm/pm.h"
 #include "rle.h"
 
-/*****************************************************************
- * TAG( match )
- *
- * Match a name against a test string for "name=value" or "name".
- * If it matches name=value, return pointer to value part, if just
- * name, return pointer to NUL at end of string.  If no match, return NULL.
- *
- * Inputs:
- *  n:  Name to match.  May also be "name=value" to make it easier
- *      to replace comments.
- *  v:  Test string.
- * Outputs:
- *  Returns pointer as above.
- * Assumptions:
- *  [None]
- * Algorithm:
- *  [None]
- */
+
+
 static const char *
 match(const char * const nArg,
       const char * const vArg) {
+/*----------------------------------------------------------------------------
+  Match a name against a test string for "name=value" or "name".
+  If it matches name=value, return pointer to value part, if just
+  name, return pointer to NUL at end of string.  If no match, return NULL.
 
+  Inputs:
+   n:  Name to match.  May also be "name=value" to make it easier
+       to replace comments.
+   v:  Test string.
+  Outputs:
+   Returns pointer as above.
+  Assumptions:
+   [None]
+  Algorithm:
+   [None]
+-----------------------------------------------------------------------------*/
     const char * n;
     const char * v;
 
@@ -70,56 +71,59 @@ match(const char * const nArg,
 
 
 
-/*****************************************************************
- * TAG( rle_putcom )
- *
- * Put a comment into the header struct.
- * Inputs:
- *  value:      Value to add to comments.
- *  the_hdr:    Header struct to add to.
- * Outputs:
- *  the_hdr:    Modified header struct.
- *  Returns previous value;
- * Assumptions:
- *  value pointer can be used as is (data is NOT copied).
- * Algorithm:
- *  Find match if any, else add at end (realloc to make bigger).
- */
 const char *
 rle_putcom(const char * const value,
-           rle_hdr *    const the_hdr) {
-
-    if ( the_hdr->comments == NULL) {
-        MALLOCARRAY_NOFAIL(the_hdr->comments, 2);
-        the_hdr->comments[0] = value;
-        the_hdr->comments[1] = NULL;
+           rle_hdr *    const hdrP) {
+/*----------------------------------------------------------------------------
+  Put a comment into the header struct, replacing the existing one
+    that has the same key, if there is one.
+  Inputs:
+   value:      Value to add to comments.
+   *hdrP:    Header struct to add to.
+  Outputs:
+   *hdrP:    Modified header struct.
+   Returns previous comment having the same key; NULL if none
+  Assumptions:
+   value pointer can be used as is (data is NOT copied).
+  Algorithm:
+   Find match if any, else add at end (realloc to make bigger).
+-----------------------------------------------------------------------------*/
+    if ( hdrP->comments == NULL) {
+        MALLOCARRAY_NOFAIL(hdrP->comments, 2);
+        hdrP->comments[0] = value;
+        hdrP->comments[1] = NULL;
     } else {
         const char ** cp;
         const char * v;
-        const char ** old_comments;
-        int i;
-        for (i = 2, cp = the_hdr->comments; *cp != NULL; ++i, ++cp)
+        const char ** oldComments;
+        unsigned int i;
+
+        for (i = 2, cp = hdrP->comments;
+             *cp != NULL && i < UINT_MAX;
+             ++i, ++cp) {
             if (match(value, *cp) != NULL) {
                 v = *cp;
                 *cp = value;
                 return v;
             }
+        }
         /* Not found */
         /* Can't realloc because somebody else might be pointing to this
          * comments block.  Of course, if this were true, then the
          * assignment above would change the comments for two headers.
-         * But at least, that won't crash the program.  Realloc will.
+         * But at least that won't crash the program.  Realloc will.
          * This would work a lot better in C++, where hdr1 = hdr2
          * could copy the pointers, too.
          */
-        old_comments = the_hdr->comments;
-        MALLOCARRAY(the_hdr->comments, i);
-        if (the_hdr->comments == NULL)
+        oldComments = hdrP->comments;
+        MALLOCARRAY(hdrP->comments, i);
+        if (hdrP->comments == NULL)
             pm_error("Unable to allocate memory for comments");
-        the_hdr->comments[--i] = NULL;
-        the_hdr->comments[--i] = value;
-        for (--i; i >= 0; --i)
-            the_hdr->comments[i] = old_comments[i];
+        assert(i >= 2);
+        hdrP->comments[--i] = NULL;
+        hdrP->comments[--i] = value;
+        for (; i > 0; --i)
+            hdrP->comments[i-1] = oldComments[i-1];
     }
 
     return NULL;