Remove RLE_CHECK_ALLOC and rle_alloc_error from URT library. This fixes a wild pointer dereference as well as cleans up code

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

4 files changed, 219 insertions, 274 deletions

diff --git a/urt/rle.h b/urt/rle.h
index 5d5ff33a..00717748 100644
--- a/urt/rle.h
+++ b/urt/rle.h
@@ -69,14 +69,6 @@ typedef unsigned short rle_map;
  */
 #define RLE_INIT_MAGIC  0x6487ED51L
 
-/*****************************************************************
- * TAG( RLE_CHECK_ALLOC )
- *
- * Test for allocation failure, scream and die if so.
- */
-#define RLE_CHECK_ALLOC( pgm, ptr, name )               \
-    ( !(ptr) ?  rle_alloc_error( pgm, name ) : 0 )
-
 /*
  * TAG( rle_hdr )
  *
@@ -163,15 +155,6 @@ extern rle_hdr rle_dflt_hdr;
 
 /* Declare RLE library routines. */
 
-/* From rle_error.c. */
-/*****************************************************************
- * TAG( rle_alloc_error )
- *
- * Print memory allocation error message and exit.
- */
-extern int rle_alloc_error( const char *pgm,
-                            const char *name );
-
 /*****************************************************************
  * TAG( rle_get_error )
  *
diff --git a/urt/rle_error.c b/urt/rle_error.c
index 23dd1fd8..801eee17 100644
--- a/urt/rle_error.c
+++ b/urt/rle_error.c
@@ -31,36 +31,6 @@
 #include "rle.h"
 
 /*****************************************************************
- * TAG( rle_alloc_error )
- *
- * Print memory allocation error message and exit.
- * Inputs:
- * 	pgm:		Name of this program.
- * 	name:		Name of memory trying to be allocated.
- * Outputs:
- * 	Prints message and exits.
- *
- * Returns int because it's used in a conditional expression.
- */
-int
-rle_alloc_error( pgm, name )
-CONST_DECL char *pgm, *name;
-{
-    if ( name )
-	fprintf( stderr, "%s: memory allocation failed.\n", pgm );
-    else
-	fprintf( stderr, "%s: memory allocation failed (no space for %s).\n",
-		 pgm, name );
-
-    exit( RLE_NO_SPACE );
-
-    /* Will some compilers bitch about this because they know exit
-     * doesn't return??
-     */
-    return 0;
-}
-
-/*****************************************************************
  * TAG( rle_get_error )
  *
  * Print an error message for the return code from rle_get_setup
@@ -74,44 +44,44 @@ CONST_DECL char *pgm, *name;
  */
 int
 rle_get_error( code, pgmname, fname )
-int code;
-CONST_DECL char *pgmname;
-CONST_DECL char *fname;
+    int code;
+    CONST_DECL char *pgmname;
+    CONST_DECL char *fname;
 {
     if (! fname || strcmp( fname, "-" ) == 0 )
-	fname = "Standard Input";
+        fname = "Standard Input";
 
     switch( code )
     {
     case RLE_SUCCESS:		/* success */
-	break;
+        break;
 
     case RLE_NOT_RLE:		/* Not an RLE file */
-	fprintf( stderr, "%s: %s is not an RLE file\n",
-		 pgmname, fname );
-	break;
+        fprintf( stderr, "%s: %s is not an RLE file\n",
+                 pgmname, fname );
+        break;
 
     case RLE_NO_SPACE:			/* malloc failed */
-	fprintf( stderr,
-		 "%s: Malloc failed reading header of file %s\n",
-		 pgmname, fname );
-	break;
+        fprintf( stderr,
+                 "%s: Malloc failed reading header of file %s\n",
+                 pgmname, fname );
+        break;
 
     case RLE_EMPTY:
-	fprintf( stderr, "%s: %s is an empty file\n",
-		 pgmname, fname );
-	break;
+        fprintf( stderr, "%s: %s is an empty file\n",
+                 pgmname, fname );
+        break;
 
     case RLE_EOF:
-	fprintf( stderr,
-		 "%s: RLE header of %s is incomplete (premature EOF)\n",
-		 pgmname, fname );
-	break;
+        fprintf( stderr,
+                 "%s: RLE header of %s is incomplete (premature EOF)\n",
+                 pgmname, fname );
+        break;
 
     default:
-	fprintf( stderr, "%s: Error encountered reading header of %s\n",
-		 pgmname, fname );
-	break;
+        fprintf( stderr, "%s: Error encountered reading header of %s\n",
+                 pgmname, fname );
+        break;
     }
     return code;
 }
diff --git a/urt/rle_getrow.c b/urt/rle_getrow.c
index 679811cc..ae220f5b 100644
--- a/urt/rle_getrow.c
+++ b/urt/rle_getrow.c
@@ -102,9 +102,13 @@ rle_get_setup(rle_hdr * const the_hdr) {
         rle_pixel * bg_color;
 
         MALLOCARRAY(the_hdr->bg_color, setup.h_ncolors);
+        if (!the_hdr->bg_color)
+            pm_error("Failed to allocation array for %u background colors",
+                     setup.h_ncolors);
         MALLOCARRAY(bg_color, 1 + (setup.h_ncolors / 2) * 2);
-        RLE_CHECK_ALLOC(the_hdr->cmd, the_hdr->bg_color && bg_color,
-                        "background color" );
+        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);
         for (i = 0; i < setup.h_ncolors; ++i)
             the_hdr->bg_color[i] = bg_color[i];
diff --git a/urt/rle_hdr.c b/urt/rle_hdr.c
index 1611324c..1edb7a3f 100644
--- a/urt/rle_hdr.c
+++ b/urt/rle_hdr.c
@@ -1,298 +1,286 @@
 /*
  * This software is copyrighted as noted below.  It may be freely copied,
- * modified, and redistributed, provided that the copyright notice is 
+ * modified, and redistributed, provided that the copyright notice is
  * preserved on all copies.
- * 
+ *
  * There is no warranty or other guarantee of fitness for this software,
  * it is provided solely "as is".  Bug reports or fixes may be sent
  * to the author, who may or may not act on them as he desires.
  *
  * You may not include this software in a program or other software product
- * without supplying the source, or without informing the end-user that the 
+ * without supplying the source, or without informing the end-user that the
  * source is available for no extra charge.
  *
  * If you modify this software, you should include a notice giving the
  * name of the person performing the modification, the date of modification,
  * and the reason for such modification.
  */
-/* 
+/*
  * rle_hdr.c - Functions to manipulate rle_hdr structures.
- * 
- * Author:	Spencer W. Thomas
- * 		EECS Dept.
- * 		University of Michigan
- * Date:	Mon May 20 1991
+ *
+ * Author:  Spencer W. Thomas
+ *      EECS Dept.
+ *      University of Michigan
+ * Date:    Mon May 20 1991
  * Copyright (c) 1991, University of Michigan
  */
 
 #include <string.h>
 
+#include "nstring.h"
+#include "mallocvar.h"
+
 #include "rle_config.h"
 #include "rle.h"
 
-/*****************************************************************
- * TAG( rle_names )
- *
+
+
+void
+rle_names(rle_hdr *    const hdrP,
+          const char * const pgmname,
+          const char * const fname,
+          int          const imgNum) {
+/*---------------------------------------------------------------------------- 
  * Load program and file names into header.
  * Inputs:
- * 	the_hdr:	Header to modify.
- * 	pgmname:	The program name.
- * 	fname:		The file name.
- * 	img_num:	Number of the image within the file.
+ *  hdrP:      Header to modify.
+ *  pgmname:   The program name.
+ *  fname:     The file name.
+ *  imgNum:    Number of the image within the file.
  * Outputs:
- * 	the_hdr:	Modified header.
- * Algorithm:
- * 	If values previously filled in (by testing is_init field),
- * 	free them.  Make copies of file name and program name,
- * 	modifying file name for standard i/o.  Set is_init field.
- */
-void
-rle_names( the_hdr, pgmname, fname, img_num )
-rle_hdr *the_hdr;
-CONST_DECL char *pgmname;
-CONST_DECL char *fname;
-int img_num;
-{
-#if 0
-    /* Can't do this because people do hdr1 = hdr2, which copies
-       the pointers. */
-
-    /* If filled in, free previous values. */
-    if ( the_hdr->is_init == RLE_INIT_MAGIC &&
-	 the_hdr->cmd != NULL && the_hdr->file_name != NULL )
-    {
-	if ( pgmname != the_hdr->cmd )
-	    free( the_hdr->cmd );
-	if ( fname != the_hdr->file_name )
-	    free( the_hdr->file_name );
-    }
-#endif
+ *  *hdrP:     Modified header.
+-----------------------------------------------------------------------------*/
+
+    /* Algorithm:
+       If values previously filled in (by testing is_init field),
+       free them.  Make copies of file name and program name,
+       modifying file name for standard i/o.  Set is_init field.
+    */
+    const char * newFname;
+    const char * newPgmname;
 
     /* Mark as filled in. */
-    the_hdr->is_init = RLE_INIT_MAGIC;
+    hdrP->is_init = RLE_INIT_MAGIC;
 
     /* Default file name for stdin/stdout. */
-    if ( fname == NULL || strcmp( fname, "-" ) == 0 || *fname == '\0' )
-	fname = "Standard I/O";
-    if ( pgmname == NULL )
-	pgmname = rle_dflt_hdr.cmd;
+    if (!fname || streq(fname, "-") || strlen(fname) == 0)
+        newFname = "Standard I/O";
+    else
+        newFname = fname;
+
+    if (pgmname)
+        newPgmname = pgmname;
+    else
+        newPgmname = rle_dflt_hdr.cmd;
 
     /* Fill in with copies of the strings. */
-    if ( the_hdr->cmd != pgmname )
-    {
-	char *tmp = (char *)malloc( strlen( pgmname ) + 1 );
-	RLE_CHECK_ALLOC( pgmname, tmp, 0 );
-	strcpy( tmp, pgmname );
-	the_hdr->cmd = tmp;
-    }
+    if (hdrP->cmd != newPgmname)
+        hdrP->cmd = pm_strdup(newPgmname);
 
-    if ( the_hdr->file_name != fname )
-    {
-	char *tmp = (char *)malloc( strlen( fname ) + 1 );
-	RLE_CHECK_ALLOC( pgmname, tmp, 0 );
-	strcpy( tmp, fname );
-	the_hdr->file_name = tmp;
-    }
+    if (hdrP->file_name != newFname)
+        hdrP->cmd = pm_strdup(newFname);
 
-    the_hdr->img_num = img_num;
+    hdrP->img_num = imgNum;
 }
 
 
+
 /* Used by rle_hdr_cp and rle_hdr_init to avoid recursion loops. */
-static int no_recurse = 0;
+static int noRecurse = 0;
 
-/*****************************************************************
- * TAG( rle_hdr_cp )
- * 
+
+
+rle_hdr *
+rle_hdr_cp(rle_hdr * const fromHdrP,
+           rle_hdr * const toHdrArgP) {
+/*----------------------------------------------------------------------------
  * Make a "safe" copy of a rle_hdr structure.
  * Inputs:
- * 	from_hdr:	Header to be copied.
+ *  *fromHdrP:   Header to be copied.
  * Outputs:
- * 	to_hdr:		Copy of from_hdr, with all memory referred to
- * 			by pointers copied.  Also returned as function
- * 			value.  If NULL, a static header is used.
+ *  *toHdrPd:    Copy of from_hdr, with all memory referred to
+ *               by pointers copied.  Also returned as function
+ *               value.  If NULL, a static header is used.
  * Assumptions:
- * 	It is safe to call rle_hdr_init on to_hdr.
- * Algorithm:
- * 	Initialize to_hdr, copy from_hdr to it, then copy the memory
- * 	referred to by all non-null pointers.
- */
-rle_hdr *
-rle_hdr_cp( from_hdr, to_hdr )
-rle_hdr *from_hdr, *to_hdr;
-{
-    static rle_hdr dflt_hdr;
-    CONST_DECL char *cmd, *file;
-    int num;
+ *  It is safe to call rle_hdr_init on *toHdrP.
+-----------------------------------------------------------------------------*/
+    /* Algorithm:
+       Initialize *toHdrP, copy *fromHdrP to it, then copy the memory
+       referred to by all non-null pointers.
+    */
+    static rle_hdr dfltHdr;
+    rle_hdr * toHdrP;
+    const char * cmd;
+    const char * file;
+    unsigned int num;
 
     /* Save command, file name, and image number if already initialized. */
-    if ( to_hdr &&  to_hdr->is_init == RLE_INIT_MAGIC )
-    {
-	cmd = to_hdr->cmd;
-	file = to_hdr->file_name;
-	num = to_hdr->img_num;
-    }
-    else
-    {
-	cmd = file = NULL;
-	num = 0;
+    if (toHdrArgP &&  toHdrArgP->is_init == RLE_INIT_MAGIC) {
+        cmd  = toHdrArgP->cmd;
+        file = toHdrArgP->file_name;
+        num  = toHdrArgP->img_num;
+    } else {
+        cmd = file = NULL;
+        num = 0;
     }
 
-    if ( !no_recurse )
-    {
-	no_recurse++;
-	rle_hdr_init( to_hdr );
-	no_recurse--;
+    if (!noRecurse) {
+        ++noRecurse;
+        rle_hdr_init(toHdrArgP);
+        --noRecurse;
     }
 
-    if ( to_hdr == NULL )
-	to_hdr = &dflt_hdr;
+    toHdrP = toHdrArgP ? toHdrArgP : &dfltHdr;
+
+    *toHdrP = *fromHdrP;
 
-    *to_hdr = *from_hdr;
+    if (toHdrP->bg_color) {
+        unsigned int i;
 
-    if ( to_hdr->bg_color )
-    {
-	int size = to_hdr->ncolors * sizeof(int);
-	to_hdr->bg_color = (int *)malloc( size );
-	RLE_CHECK_ALLOC( to_hdr->cmd, to_hdr->bg_color, "background color" );
-	memcpy( to_hdr->bg_color, from_hdr->bg_color, size );
+        MALLOCARRAY(toHdrP->bg_color, toHdrP->ncolors);
+        if (!toHdrP->bg_color)
+            pm_error("Failed to allocate array for %u background colors",
+                     toHdrP->ncolors);
+        for (i = 0; i < toHdrP->ncolors; ++i)
+            toHdrP->bg_color[i] = fromHdrP->bg_color[i];
     }
 
-    if ( to_hdr->cmap )
-    {
-	int size = to_hdr->ncmap * (1 << to_hdr->cmaplen) * sizeof(rle_map);
-	to_hdr->cmap = (rle_map *)malloc( size );
-	RLE_CHECK_ALLOC( to_hdr->cmd, to_hdr->cmap, "color map" );
-	memcpy( to_hdr->cmap, from_hdr->cmap, size );
+    if (toHdrP->cmap) {
+        size_t const size =
+            toHdrP->ncmap * (1 << toHdrP->cmaplen) * sizeof(rle_map);
+        toHdrP->cmap = malloc(size);
+        if (!toHdrP->cmap)
+            pm_error("Failed to allocate memory for %u color maps "
+                     "of length %u", toHdrP->ncmap, 1 << toHdrP->cmaplen);
+        memcpy(toHdrP->cmap, fromHdrP->cmap, size);
     }
 
     /* Only copy array of pointers, as the original comment memory
      * never gets overwritten.
      */
-    if ( to_hdr->comments )
-    {
-	int size = 0;
-	CONST_DECL char **cp;
-	for ( cp=to_hdr->comments; *cp; cp++ )
-	    size++;		/* Count the comments. */
-	/* Check if there are really any comments. */
-	if ( size )
-	{
-	    size++;		/* Copy the NULL pointer, too. */
-	    size *= sizeof(char *);
-	    to_hdr->comments = (CONST_DECL char **)malloc( size );
-	    RLE_CHECK_ALLOC( to_hdr->cmd, to_hdr->comments, "comments" );
-	    memcpy( to_hdr->comments, from_hdr->comments, size );
-	}
-	else
-	    to_hdr->comments = NULL;	/* Blow off empty comment list. */
+    if (toHdrP->comments) {
+        unsigned int  size;
+        const char ** cp;
+
+        /* Count the comments. */
+        for (cp = toHdrP->comments, size = 0; *cp; ++cp)
+            ++size;
+
+        /* Check if there are really any comments. */
+        if (size > 0) {
+            ++size;     /* Copy the NULL pointer, too. */
+            size *= sizeof(char *);
+            toHdrP->comments = malloc(size);
+            if (!toHdrP->comments)
+                pm_error("Failed to allocation %u bytes for comments", size);
+            memcpy(toHdrP->comments, fromHdrP->comments, size);
+        } else
+            toHdrP->comments = NULL;    /* Blow off empty comment list. */
     }
 
     /* Restore the names to their original values. */
-    to_hdr->cmd = cmd;
-    to_hdr->file_name = file;
+    toHdrP->cmd       = cmd;
+    toHdrP->file_name = file;
 
     /* Lines above mean nothing much happens if cmd and file are != NULL. */
-    rle_names( to_hdr, to_hdr->cmd, to_hdr->file_name, num );
+    rle_names(toHdrP, toHdrP->cmd, toHdrP->file_name, num);
 
-    return to_hdr;
+    return toHdrP;
 }
 
-/*****************************************************************
- * TAG( rle_hdr_clear )
- * 
+
+
+void
+rle_hdr_clear(rle_hdr * const hdrP) {
+/*----------------------------------------------------------------------------
  * Clear out the allocated memory pieces of a header.
  *
  * This routine is intended to be used internally by the library, to
  * clear a header before putting new data into it.  It clears all the
  * fields that would be set by reading in a new image header.
  * Therefore, it does not clear the program and file names.
- * 
+ *
  * Inputs:
- * 	the_hdr:	To be cleared.
+ *  hdrP:    To be cleared.
  * Outputs:
- * 	the_hdr:	After clearing.
+ *  *hdrP:   After clearing.
  * Assumptions:
- * 	If is_init field is RLE_INIT_MAGIC, the header has been
- * 	properly initialized.  This will fail every 2^(-32) times, on
- * 	average.
- * Algorithm:
- * 	Free memory and set to zero all pointers, except program and
- * 	file name.
- */
-void
-rle_hdr_clear( the_hdr )
-rle_hdr *the_hdr;
-{
+ *  If is_init field is RLE_INIT_MAGIC, the header has been
+ *  properly initialized.  This will fail every 2^(-32) times, on
+ *  average.
+-----------------------------------------------------------------------------*/
+    /* Algorithm:
+       Free memory and set to zero all pointers, except program and
+       file name.
+    */
+
     /* Try to free memory.  Assume if is_init is properly set that this
      * header has been previously initialized, therefore it is safe to
      * free memory.
      */
-    if ( the_hdr && the_hdr->is_init == RLE_INIT_MAGIC )
-    {
-	if ( the_hdr->bg_color )
-	    free( the_hdr->bg_color );
-	the_hdr->bg_color = 0;
-	if ( the_hdr->cmap )
-	    free( the_hdr->cmap );
-	the_hdr->cmap = 0;
-	/* Unfortunately, we don't know how to free the comment memory. */
-	if ( the_hdr->comments )
-	    free( the_hdr->comments );
-	the_hdr->comments = 0;
+    if (hdrP && hdrP->is_init == RLE_INIT_MAGIC) {
+        if (hdrP->bg_color )
+            free(hdrP->bg_color);
+        hdrP->bg_color = NULL;
+        if (hdrP->cmap )
+            free(hdrP->cmap);
+        hdrP->cmap = NULL;
+        /* Unfortunately, we don't know how to free the comment memory. */
+        if (hdrP->comments)
+            free(hdrP->comments);
+        hdrP->comments = NULL;
     }
 }
 
 
 
-/*****************************************************************
- * TAG( rle_hdr_init )
- * 
+rle_hdr *
+rle_hdr_init(rle_hdr * const hdrP) {
+/*----------------------------------------------------------------------------
  * Initialize a rle_hdr structure.
  * Inputs:
- * 	the_hdr:	Header to be initialized.
+ *  hdrP:    Header to be initialized.
  * Outputs:
- * 	the_hdr:	Initialized header.
+ *  *hdrP:   Initialized header.
  * Assumptions:
- * 	If the_hdr->is_init is RLE_INIT_MAGIC, the header has been
- * 	previously initialized.
- * 	If the_hdr is a copy of another rle_hdr structure, the copy
- * 	was made with rle_hdr_cp.
- * Algorithm:
- *  Fill in fields of rle_dflt_hdr that could not be set by the loader
- *	If the_hdr is rle_dflt_hdr, do nothing else
- *  Else:
- *	  If the_hdr is NULL, return a copy of rle_dflt_hdr in static storage
- * 	  If the_hdr->is_init is RLE_INIT_MAGIC, free all memory
- * 	     pointed to by non-null pointers.
- *    If this is a recursive call to rle_hdr_init, clear *the_hdr and
- *      return the_hdr.
- *    Else make a copy of rle_dflt_hdr and return its address.  Make the
- *      copy in static storage if the_hdr is NULL, and in the_hdr otherwise.
- */
-rle_hdr *
-rle_hdr_init( the_hdr )
-rle_hdr *the_hdr;
-{
-    rle_hdr *ret_hdr;
+ *  If hdrP->is_init is RLE_INIT_MAGIC, the header has been
+ *  previously initialized.
+ *  If the_hdr is a copy of another rle_hdr structure, the copy
+ *  was made with rle_hdr_cp.
+-----------------------------------------------------------------------------*/
+    /* Algorithm:
+       Fill in fields of rle_dflt_hdr that could not be set by the loader
+       If the_hdr is rle_dflt_hdr, do nothing else
+       Else:
+         If hdrP is NULL, return a copy of rle_dflt_hdr in static storage
+         If hdrP->is_init is RLE_INIT_MAGIC, free all memory
+            pointed to by non-null pointers.
+         If this is a recursive call to rle_hdr_init, clear *hdrP and
+           return hdrP.
+         Else make a copy of rle_dflt_hdr and return its address.  Make the
+           copy in static storage if hdrP is NULL, and in *hdrP otherwise.
+    */
+    rle_hdr * retval;
 
     rle_dflt_hdr.rle_file = stdout;
-    /* The rest of rle_dflt_hdr is set by the loader's data initialization */
 
-    if ( the_hdr == &rle_dflt_hdr )
-	return the_hdr;
+    /* The rest of rle_dflt_hdr is set by the loader's data initialization */
 
-    rle_hdr_clear( the_hdr );
+    if (hdrP == &rle_dflt_hdr)
+        retval = hdrP;
+    else {
+        rle_hdr_clear(hdrP);
 
-    /* Only call rle_hdr_cp if not called from there. */
-    if ( !no_recurse )
-    {
-	no_recurse++;
-	ret_hdr = rle_hdr_cp( &rle_dflt_hdr, the_hdr );
-	no_recurse--;
+        /* Call rle_hdr_cp only if not called from there. */
+        if (!noRecurse) {
+            ++noRecurse;
+            retval = rle_hdr_cp(&rle_dflt_hdr, hdrP);
+            --noRecurse;
+        } else
+            retval = hdrP;
     }
-    else
-	ret_hdr = the_hdr;
-
-    return ret_hdr;
+    return retval;
 }
+
+
+