[PATCH 20/20] tools/build/*.c: Clean up issues reported by CodeSonar

Gedare Bloom gedare at rtems.org
Wed Nov 26 15:31:10 UTC 2014


I did not look at this, but in general we should consider to relocate
the tools subdirectory to a different repo.

On Tue, Nov 25, 2014 at 6:02 PM, Joel Sherrill
<joel.sherrill at oarcorp.com> wrote:
> From: Josh Oguin <josh.oguin at oarcorp.com>
>
> This code is built without warnings and ignored by Coverity Scan.
> CodeSonar found a wide range of issues including buffer overruns,
> buffer underruns, questionable type conversions, leaks, etc. This
> set of patches addresses all reported issues.
> ---
>  tools/build/binpatch.c    | 144 ++++++++++++++--------------
>  tools/build/cklength.c    |  28 +++---
>  tools/build/eolstrip.c    |  14 ++-
>  tools/build/rtems-bin2c.c | 233 ++++++++++++++++++++++++----------------------
>  tools/build/unhex.c       | 196 +++++++++++++++++++-------------------
>  5 files changed, 309 insertions(+), 306 deletions(-)
>
> diff --git a/tools/build/binpatch.c b/tools/build/binpatch.c
> index 4748b11..d85303e 100644
> --- a/tools/build/binpatch.c
> +++ b/tools/build/binpatch.c
> @@ -1,11 +1,10 @@
> -
> -#include <stdio.h>
> -#include <stdlib.h>
> -
>  /*
>   * This function will patch binary file
>   */
>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <assert.h>
>
>  static char buf[512];
>
> @@ -25,128 +24,121 @@ usage(void)
>    printf("<byte1>  - byte 1 of patch\n");
>    printf("<byte2>  - byte 1 of patch\n");
>    printf("<byte3>  - byte 1 of patch\n");
> -
> -  return;
>  }
>
>  int
>  main(int argc, char **argv)
>  {
> -  int   c;
> +  int    c;
>    FILE  *ofp, *ifp;
> -  char  patch[4], *end;
> -  int   patchLen, tmp, i, off, cnt, patched, len, reloc;
> -
> +  char  *patch, *end;
> +  int    patchLen, tmp, i, off, cnt, patched, len, reloc;
>
>    /* parse command line options */
> -  while ((c = getopt(argc, argv, "h")) >= 0)
> -    {
> -      switch (c)
> -       {
> -       case 'h':
> -         usage();
> -         return 0;
> -       default:
> -         usage();
> -         return 1;
> -       }
> -    }
> -
> -  if(argc < 6)
> -    {
> +  while ((c = getopt(argc, argv, "h")) >= 0) {
> +      switch (c) {
> +        case 'h':
> +          usage();
> +          return 0;
> +        default:
> +          usage();
> +          return 1;
> +        }
> +  }
> +
> +  if(argc < 6) {
>        usage();
>        return 1;
> -    }
> +  }
>
>    /* Let us get offset in file */
>    reloc = strtol(argv[3], &end, 0);
> -  if(end == argv[3] || off < 0)
> -    {
> +  if (end == argv[3] || reloc < 0) {
>        fprintf(stderr, "bad reloc value %s\n", argv[3]);
>        return 1;
> -    }
> +  }
>
>    off = strtol(argv[4], &end, 0);
> -  if(end == argv[4] || off < 0 || off < reloc)
> -    {
> +  if (end == argv[4] || off < 0 || off < reloc) {
>        fprintf(stderr, "bad offset value %s\n", argv[4]);
>        return 1;
> -    }
> +  }
>
>    off -= reloc;
>
>    /* Let us get patch  */
>    patchLen = argc - 5;
> +  patch = calloc( patchLen, sizeof(char) );
> +  assert( patch );
>
> -  for(i=0; i<patchLen; i++)
> -    {
> +  for (i=0; i<patchLen; i++) {
>        tmp = strtol(argv[5+i], &end, 0);
>
> -      if(end == argv[4+i] || tmp < 0 || tmp > 0xff)
> -       {
> -         fprintf(stderr, "bad byte value %s\n", argv[5+i]);
> -         return 1;
> -       }
> -      patch[i] = tmp;
> -    }
> +      if (end == argv[4+i]) {
> +          fprintf(stderr, "bad byte value %s\n", argv[5+i]);
> +          free(patch);
> +          return 1;
> +      }
> +      if (tmp < 0 || tmp > 0xff) {
> +          fprintf(stderr, "not a byte value %s\n", argv[5+i]);
> +          free(patch);
> +          return 1;
> +      }
> +      patch[i] = (tmp & 0xff);
> +  }
>
>    ifp = fopen(argv[2], "r");
> -  if(ifp == NULL)
> -    {
> +  if (ifp == NULL) {
>        fprintf(stderr, "unable to open file %s\n", argv[2]);
> +      free(patch);
>        return 1;
> -    }
> +  }
>
>    ofp = fopen(argv[1], "w");
> -  if(ofp == NULL)
> -    {
> +  if (ofp == NULL) {
>        fprintf(stderr, "unable to open file %s\n", argv[1]);
> +      fclose(ifp);
> +      free(patch);
>        return 1;
> -    }
> +  }
>
>    cnt     = 0;
>    patched = 0;
> -  for(;;)
> -    {
> +  for(;;) {
>        len = fread(buf, 1, sizeof(buf), ifp);
>
> -      if(len == 0)
> -       {
> -         break;
> -       }
> -
> -      if(cnt <= off && (cnt + len) > off)
> -       {
> -         /* Perform patch */
> -         for(i=0; i<patchLen && (off+i)<(cnt+len); i++)
> -           {
> -             buf[off-cnt+i] = patch[i];
> -           }
> -         patched = 1;
> -       }
> -      else if(cnt > off && cnt < (off + patchLen))
> -       {
> -         /* Perform patch */
> -         for(i=cnt-off; i<patchLen; i++)
> -           {
> -             buf[off-cnt+i] = patch[i];
> -           }
> -         patched = 1;
> -       }
> +      if(len == 0) {
> +          break;
> +      }
> +
> +      if (cnt <= off && (cnt + len) > off) {
> +          /* Perform patch */
> +          for(i=0; i<patchLen && (off+i)<(cnt+len); i++)
> +            {
> +              buf[off-cnt+i] = patch[i];
> +            }
> +          patched = 1;
> +      } else if(cnt > off && cnt < (off + patchLen)) {
> +          /* Perform patch */
> +          for(i=cnt-off; i<patchLen; i++) {
> +              buf[off-cnt+i] = patch[i];
> +          }
> +          patched = 1;
> +      }
>
>        fwrite(buf, 1, len, ofp);
>
>        cnt += len;
> -    }
> +  }
>
>    fclose(ifp);
>    fclose(ofp);
> +  free(patch);
>
> -  if(!patched)
> -    {
> +  if (!patched) {
>        fprintf(stderr, "warning: offset is beyond input file length\n");
>        fprintf(stderr, "         no patch is performed\n");
> -    }
> +  }
>
>    return 0;
>  }
> diff --git a/tools/build/cklength.c b/tools/build/cklength.c
> index 47675ce..fd6ed26 100644
> --- a/tools/build/cklength.c
> +++ b/tools/build/cklength.c
> @@ -125,8 +125,7 @@ int main(
>
>      opterr = 0;                                 /* we'll report all errors */
>      while ((c = getopt(argc, argv, GETOPTARGS)) != EOF)
> -        switch (c)
> -        {
> +        switch (c) {
>              case 'l':                            /* line length */
>                  line_length = atoi( optarg );
>                  if ( line_length < 0 || line_length > BUFFER_SIZE )
> @@ -150,8 +149,7 @@ int main(
>                  showusage = TRUE;
>          }
>
> -    if (showusage)
> -    {
> +    if (showusage) {
>          (void) fprintf(stderr, "%s", USAGE);
>          exit(1);
>      }
> @@ -160,9 +158,10 @@ int main(
>       *  traverse and process the arguments
>       */
>
> -    for ( ; argv[optind]; optind++)
> +    for ( ; optind < argc; optind++) {
>          if (Failed(process(argv[optind])))
>              rc = FAILURE;
> +    }
>
>      return rc;
>  }
> @@ -268,15 +267,11 @@ error(int error_flag, ...)
>
>      (void) fflush(stderr);
>
> -    if (error_flag & (ERR_FATAL | ERR_ABORT))
> -    {
> -        if (error_flag & ERR_FATAL)
> -        {
> +    if (error_flag & (ERR_FATAL | ERR_ABORT)) {
> +        if (error_flag & ERR_FATAL) {
>              error(0, "fatal error, exiting");
>              exit(local_errno ? local_errno : 1);
> -        }
> -        else
> -        {
> +        } else {
>              error(0, "fatal error, aborting");
>              abort();
>          }
> @@ -291,19 +286,18 @@ getparm(char *s,
>  {
>      long val;
>
> -    if ( ! strchr("0123456789-", *s))
> -    {
> +    if ( !strchr("0123456789-", *s) ) {
>          error(ERR_FATAL, "'%s' is not a number", s);
> -        return min;
> +        /* does not return */
>      }
>
>      val = strtol(s, (char **) NULL, 0);
> -    if ((val < min) || (val > max))
> -    {
> +    if ((val < min) || (val > max)) {
>          if (min == max)
>              error(ERR_FATAL, "%s can only be %ld", s, min);
>          else
>              error(ERR_FATAL, "%s must be between %ld and %ld", msg, min, max);
> +        /* does not return */
>      }
>
>      return val;
> diff --git a/tools/build/eolstrip.c b/tools/build/eolstrip.c
> index 859f174..e93b45e 100644
> --- a/tools/build/eolstrip.c
> +++ b/tools/build/eolstrip.c
> @@ -189,13 +189,14 @@ process(char *arg)
>      /*
>       *  Don't count the carriage return.
>       */
> -
> -    length = strlen( buffer ) - 1;
> +    length = 0;
> +    if ( *buffer != '\0' )
> +      length = strnlen( buffer, BUFFER_SIZE ) - 1;
>
>      if ( buffer[ length ] != '\n' )
>        error(ERR_ERRNO|ERR_FATAL, "Line %d too long in %s\n", line_number, arg);
>
> -    while ( isspace( (unsigned char) buffer[ length ] ) )
> +    while ( length && isspace( (unsigned char) buffer[ length ] ) )
>        buffer[ length-- ] = '\0';
>
>      if ( test_only ) {
> @@ -208,8 +209,11 @@ process(char *arg)
>
>    fclose( in );
>    if ( !test_only ) {
> -    fclose( out );
> -    rename( outname, arg );
> +    if (out) fclose( out );
> +    rc = rename( outname, arg );
> +    if ( rc != 0 ) {
> +      fprintf( stderr, "Unable to rename %s to %s\n", outname, arg );
> +    }
>    }
>    return rc;
>  }
> diff --git a/tools/build/rtems-bin2c.c b/tools/build/rtems-bin2c.c
> index 710c97e..fc0381a 100644
> --- a/tools/build/rtems-bin2c.c
> +++ b/tools/build/rtems-bin2c.c
> @@ -55,13 +55,16 @@ int myfgetc(FILE *f)
>  void process(const char *ifname, const char *ofname)
>  {
>    FILE *ifile, *ocfile, *ohfile;
> -  char buf[PATH_MAX], *p;
> -  char obasename[PATH_MAX];
> -  char ocname[PATH_MAX];
> -  char ohname[PATH_MAX];
> +  char buf[PATH_MAX+1], *p;
> +  char obasename[PATH_MAX+1];
> +  char ocname[PATH_MAX+1];
> +  char ohname[PATH_MAX+1];
>    const char *cp;
>    size_t len;
>
> +  ocfile = NULL;
> +  ohfile = NULL;
> +
>    /* Error check */
>    if ( !ifname || !ofname ) {
>      fprintf(stderr, "process has NULL filename\n");
> @@ -100,75 +103,84 @@ void process(const char *ifname, const char *ofname)
>    }
>
>    if ( createC ) {
> -  ocfile = fopen(ocname, "wb");
> -  if (ocfile == NULL) {
> -    fprintf(stderr, "cannot open %s for writing\n", ocname);
> -    exit(1);
> -  }
> +    ocfile = fopen(ocname, "wb");
> +    if (ocfile == NULL) {
> +      fprintf(stderr, "cannot open %s for writing\n", ocname);
> +      exit(1);
> +    }
>    }
>
>    if ( createH ) {
> -  ohfile = fopen(ohname, "wb");
> -  if (ohfile == NULL) {
> -    fprintf(stderr, "cannot open %s for writing\n", ohname);
> -    exit(1);
> -  }
> +    ohfile = fopen(ohname, "wb");
> +    if (ohfile == NULL) {
> +      fprintf(stderr, "cannot open %s for writing\n", ohname);
> +      exit(1);
> +    }
>    }
>
>    /* find basename */
>    char *ifbasename = strdup(ifname);
> +  if ( ifbasename == NULL ) {
> +    fprintf(stderr, "cannot allocate memory\n" );
> +    fclose(ifile);
> +    if ( createC ) { fclose(ocfile); }
> +    if ( createH ) { fclose(ohfile); }
> +    exit(1);
> +  }
> +
>    ifbasename = basename(ifbasename);
>
>    strcpy(buf, ifbasename);
> -  for (p = buf; *p != '\0'; ++p)
> -    if (!isalnum(*p))
> +  for (p = buf; *p != '\0'; ++p) {
> +    if (!isalnum((unsigned char)*p)) /* cast to avoid negative indexing */
>        *p = '_';
> +  }
>
>    if ( createC ) {
> -  /* print C file header */
> -  fprintf(
> -    ocfile,
> -    "/*\n"
> -    " *  Declarations for C structure representing binary file %s\n"
> -    " *\n"
> -    " *  WARNING: Automatically generated -- do not edit!\n"
> -    " */\n"
> -    "\n"
> -    "#include <sys/types.h>\n"
> -    "\n",
> -    ifbasename
> -  );
> +    /* print C file header */
> +    fprintf(
> +      ocfile,
> +      "/*\n"
> +      " *  Declarations for C structure representing binary file %s\n"
> +      " *\n"
> +      " *  WARNING: Automatically generated -- do not edit!\n"
> +      " */\n"
> +      "\n"
> +      "#include <sys/types.h>\n"
> +      "\n",
> +      ifbasename
> +    );
>
> -  /* print structure */
> -  fprintf(
> -    ocfile,
> -    "%s%sunsigned char %s[] = {\n  ",
> -    ((usestatic) ? "static " : ""),
> -    ((useconst) ? "const " : ""),
> -    buf
> -  );
> -  int c, col = 1;
> -  while ((c = myfgetc(ifile)) != EOF) {
> -    if (col >= 78 - 6) {
> -      fprintf(ocfile, "\n  ");
> -      col = 1;
> -    }
> -    fprintf(ocfile, "0x%.2x, ", c);
> -    col += 6;
> +    /* print structure */
> +    fprintf(
> +      ocfile,
> +      "%s%sunsigned char %s[] = {\n  ",
> +      ((usestatic) ? "static " : ""),
> +      ((useconst) ? "const " : ""),
> +      buf
> +    );
> +    int c, col = 1;
> +    while ((c = myfgetc(ifile)) != EOF) {
> +      if (col >= 78 - 6) {
> +        fprintf(ocfile, "\n  ");
> +        col = 1;
> +      }
> +      fprintf(ocfile, "0x%.2x, ", c);
> +      col += 6;
>
> -  }
> -  fprintf(ocfile, "\n};\n");
> +    }
> +    fprintf(ocfile, "\n};\n");
>
> -  /* print sizeof */
> -  fprintf(
> -    ocfile,
> -    "\n"
> -    "%s%ssize_t %s_size = sizeof(%s);\n",
> -    ((usestatic) ? "static " : ""),
> -    ((useconst) ? "const " : ""),
> -    buf,
> -    buf
> -  );
> +    /* print sizeof */
> +    fprintf(
> +      ocfile,
> +      "\n"
> +      "%s%ssize_t %s_size = sizeof(%s);\n",
> +      ((usestatic) ? "static " : ""),
> +      ((useconst) ? "const " : ""),
> +      buf,
> +      buf
> +    );
>    } /* createC */
>
>    /*****************************************************************/
> @@ -176,61 +188,61 @@ void process(const char *ifname, const char *ofname)
>    /*****************************************************************/
>
>    if ( createH ) {
> -  /* print H file header */
> -  char hbasename[PATH_MAX];
> -  char* p;
> -  /* Clean up the file name if it is an abs path */
> -  strcpy(
> -    hbasename,
> -    obasename
> -  );
> -  p = hbasename;
> -  while (*p != '\0') {
> -    if (*p < '0' || *p > 'z')
> -      *p = '_';
> -    ++p;
> -  }
> -  fprintf(
> -    ohfile,
> -    "/*\n"
> -    " *  Extern declarations for C structure representing binary file %s\n"
> -    " *\n"
> -    " *  WARNING: Automatically generated -- do not edit!\n"
> -    " */\n"
> -    "\n"
> -    "#ifndef __%s_h\n"
> -    "#define __%s_h\n"
> -    "\n"
> -    "#include <sys/types.h>\n"
> -    "\n",
> -    ifbasename,  /* header */
> -    hbasename,  /* ifndef */
> -    hbasename   /* define */
> -  );
> +    /* print H file header */
> +    char hbasename[PATH_MAX];
> +    char* p;
> +    /* Clean up the file name if it is an abs path */
> +    strcpy(
> +      hbasename,
> +      obasename
> +    );
> +    p = hbasename;
> +    while (*p != '\0') {
> +      if (*p < '0' || *p > 'z')
> +        *p = '_';
> +      ++p;
> +    }
> +    fprintf(
> +      ohfile,
> +      "/*\n"
> +      " *  Extern declarations for C structure representing binary file %s\n"
> +      " *\n"
> +      " *  WARNING: Automatically generated -- do not edit!\n"
> +      " */\n"
> +      "\n"
> +      "#ifndef __%s_h\n"
> +      "#define __%s_h\n"
> +      "\n"
> +      "#include <sys/types.h>\n"
> +      "\n",
> +      ifbasename,  /* header */
> +      hbasename,  /* ifndef */
> +      hbasename   /* define */
> +    );
>
> -  /* print structure */
> -  fprintf(
> -    ohfile,
> -    "extern %s%sunsigned char %s[];",
> -    ((usestatic) ? "static " : ""),
> -    ((useconst) ? "const " : ""),
> -    buf
> -  );
> -  /* print sizeof */
> -  fprintf(
> -    ohfile,
> -    "\n"
> -    "extern %s%ssize_t %s_size;\n",
> -    ((usestatic) ? "static " : ""),
> -    ((useconst) ? "const " : ""),
> -    buf
> -  );
> +    /* print structure */
> +    fprintf(
> +      ohfile,
> +      "extern %s%sunsigned char %s[];",
> +      ((usestatic) ? "static " : ""),
> +      ((useconst) ? "const " : ""),
> +      buf
> +    );
> +    /* print sizeof */
> +    fprintf(
> +      ohfile,
> +      "\n"
> +      "extern %s%ssize_t %s_size;\n",
> +      ((usestatic) ? "static " : ""),
> +      ((useconst) ? "const " : ""),
> +      buf
> +    );
>
> -  fprintf(
> -    ohfile,
> -    "\n"
> -    "#endif\n"
> -  );
> +    fprintf(
> +      ohfile,
> +      "\n"
> +      "#endif\n"
> +    );
>    } /* createH */
>
>    /*****************************************************************/
> @@ -240,6 +252,7 @@ void process(const char *ifname, const char *ofname)
>    fclose(ifile);
>    if ( createC ) { fclose(ocfile); }
>    if ( createH ) { fclose(ohfile); }
> +  free(ifbasename);
>  }
>
>  void usage(void)
> diff --git a/tools/build/unhex.c b/tools/build/unhex.c
> index be4465d..7c3bab0 100644
> --- a/tools/build/unhex.c
> +++ b/tools/build/unhex.c
> @@ -36,6 +36,7 @@ usage:    unhex [-va] [ -o file ] [ file [file ... ] ]\n\
>  #include <stdlib.h>
>  #include <stdarg.h>
>  #include <errno.h>
> +#include <assert.h>
>
>  #include "config.h"
>
> @@ -71,10 +72,10 @@ typedef unsigned long u32;
>   * Pick out designated bytes
>   */
>
> -#define B0(x)       ((x) & 0xff)
> -#define B1(x)       B0((x) >> 8)
> -#define B2(x)       B0((x) >> 16)
> -#define B3(x)       B0((x) >> 24)
> +#define B0(x)       (((u32)x) & 0xff)
> +#define B1(x)       B0(((u32)x) >> 8)
> +#define B2(x)       B0(((u32)x) >> 16)
> +#define B3(x)       B0(((u32)x) >> 24)
>
>  typedef struct buffer_rec {
>      u32 dl_destaddr;
> @@ -119,9 +120,10 @@ int   getnibble(char **p);
>  int   getbyte(char **p);
>  long  getNbytes(char **p, int n);
>  void  badformat(char *s, char *fname, char *msg);
> +void  fix_string_from_gets(char *s, size_t max);
>
> -#define get1bytes(p)    ((int) getbyte(p))
> -#define get2bytes(p)    ((int) getNbytes(p, 2))
> +#define get1bytes(p)    (getbyte(p))
> +#define get2bytes(p)    getNbytes(p, 2)
>  #define get3bytes(p)    getNbytes(p, 3)
>  #define get4bytes(p)    getNbytes(p, 4)
>
> @@ -140,15 +142,15 @@ int main(
>    char **argv
>  )
>  {
> -    register int c;
> +    int c;
>      bool showusage = FALSE;                     /* usage error? */
>      int rc = 0;
>      FILE *outfp, *infp;
> +    long base_long = 0;
>
>      /*
>       * figure out invocation leaf-name
>       */
> -
>      if ((progname = strrchr(argv[0], '/')) == (char *) NULL)
>          progname = argv[0];
>      else
> @@ -159,13 +161,18 @@ int main(
>      /*
>       *  Check options and arguments.
>       */
> -
>      progname = argv[0];
> -    while ((c = getopt(argc, argv, "F:a:o:vl")) != EOF)
> -        switch (c)
> -        {
> +    while ((c = getopt(argc, argv, "F:a:o:vl")) != EOF) {
> +        switch (c) {
>              case 'a':                           /* base address */
> -                base = stol(optarg);
> +                base_long = stol(optarg);
> +                if (base_long < 0) {
> +                    (void) fprintf(stderr, "%s", USAGE);
> +                    (void) fprintf(stderr, "\n*** base is an illegal value\n" );
> +                    exit(1);
> +                }
> +                base = (u32) base_long;
> +
>                  break;
>
>              case 'l':                           /* linear output */
> @@ -187,53 +194,47 @@ int main(
>              case '?':
>                  showusage = TRUE;
>          }
> +    }
>
> -    if (showusage)
> -    {
> +    if (showusage) {
>          (void) fprintf(stderr, "%s", USAGE);
>          exit(1);
>      }
>
> -    if (linear && (base != 0))
> -    {
> +    if (linear && (base != 0)) {
>          error(0, "-l and -a may not be specified in combination");
>          exit(1);
>      }
>
> -    if (STREQ(outfilename, "-"))
> -    {
> +    if (STREQ(outfilename, "-")) {
>          outfp = stdout;
>          outfilename = "stdout";
> +    } else if ((outfp = fopen(outfilename, "w")) == (FILE *) NULL) {
> +        error(-1, "couldn't open '%s' for output", outfilename);
> +        exit(1);
>      }
> -    else
> -        if ((outfp = fopen(outfilename, "w")) == (FILE *) NULL)
> -        {
> -            error(-1, "couldn't open '%s' for output", outfilename);
> -            exit(1);
> -        }
>
>      /*
>       * Now process the input files (or stdin, if none specified)
>       */
> -
> -    if (argv[optind] == (char *) NULL)          /* just stdin */
> +    if (argc == optind)          /* just stdin */
>          exit(unhex(stdin, "stdin", outfp, outfilename));
>      else
> -        for (; (optarg = argv[optind]); optind++)
> -        {
> -            if (STREQ(optarg, "-"))
> +        for (; optind < argc ; optind++) {
> +            optarg = argv[optind];
> +            if (STREQ(optarg, "-")) {
>                  rc += unhex(stdin, "stdin", outfp, outfilename);
> -            else
> -            {
> -                if ((infp = fopen(optarg, "r")) == (FILE *) NULL)
> -                {
> +            } else {
> +                if ((infp = fopen(optarg, "r")) == (FILE *) NULL) {
>                      error(-1, "couldn't open '%s' for input", optarg);
>                      exit(1);
>                  }
>                  rc += unhex(infp, optarg, outfp, outfilename);
> +                fclose(infp);
>              }
>          }
>
> +    fclose(outfp);
>      return(rc);
>  }
>
> @@ -260,8 +261,7 @@ unhex(FILE *ifp,
>       *  'stripffs'
>       */
>
> -    if (FFfill)
> -    {
> +    if (FFfill) {
>          (void) fseek(ofp, 0, 0);
>          for (c = FFfill; c > 0; c--)
>              (void) fputc(0xFF, ofp);
> @@ -271,11 +271,9 @@ unhex(FILE *ifp,
>       * Read the first char from file and determine record types
>       */
>
> -    if ((c = getc(ifp)) != EOF)
> -    {
> +    if ((c = getc(ifp)) != EOF) {
>          ungetc(c, ifp);
> -        switch(c)
> -        {
> +        switch(c) {
>              case 'S':
>                  convert_S_records(ifp, inm, ofp, onm);
>                  break;
> @@ -292,7 +290,8 @@ unhex(FILE *ifp,
>              default:
>              {
>                  char tmp[2];
> -                tmp[0] = c; tmp[1] = 0;
> +                tmp[0] = (char) (c & 0x7f);
> +                tmp[1] = 0;
>                  badformat(tmp, inm, BADFMT);
>              }
>          }
> @@ -317,21 +316,17 @@ convert_Intel_records(
>      int incksum;
>      int c;
>      int rectype;                    /* record type */
> -    int len;                        /* data length of current line */
> +    long len;                        /* data length of current line */
>      u32 addr;
>      u32 base_address = 0;
>      bool endrecord = FALSE;
>      buffer_rec tb;
> +    long getBytesStatus;
>
> -    while ( ! endrecord && (fgets(buff, sizeof(buff), ifp)))
> -    {
> +    while ( !endrecord && (fgets(buff, sizeof(buff), ifp)) ) {
>          p = &buff[0];
>
> -        if (p[strlen(p)-1] == '\n')                 /* get rid of newline */
> -            p[strlen(p)-1] = '\0';
> -
> -        if (p[strlen(p)-1] == '\r')                 /* get rid of any CR */
> -            p[strlen(p)-1] = '\0';
> +        fix_string_from_gets(p, sizeof(buff));
>
>          tb.dl_count = 0;
>
> @@ -342,17 +337,17 @@ convert_Intel_records(
>          if ((len = getbyte(&p)) == -1)      /* record len */
>              badformat(buff, inm, BADLEN);
>
> -        if ((addr = get2bytes(&p)) == -1L)          /* record addr */
> +        if ((getBytesStatus = get2bytes(&p)) == -1L)          /* record addr */
>              badformat(buff, inm, BADADDR);
> +        addr = (u32) getBytesStatus;
>
>          rectype = getbyte(&p);
>
>          cksum = len + B0(addr) + B1(addr) + rectype;
>
> -        switch (rectype)
> -        {
> +        switch (rectype) {
>              case 0x00:                  /* normal data record */
> -                tb.dl_destaddr = base_address + addr;
> +                tb.dl_destaddr = (u32) base_address + addr;
>                  while (len--)
>                  {
>                      if ((c = getbyte(&p)) == -1)
> @@ -369,15 +364,18 @@ convert_Intel_records(
>                  break;
>
>              case 0x02:                  /* new base */
> -                if ((base_address = get2bytes(&p)) == -1L)
> +            {
> +                long blong;
> +                if ((blong = get2bytes(&p)) == -1L)
>                      badformat(buff, inm, BADBASE);
> +                base_address = (u32) blong;
>                  cksum += B0(base_address) + B1(base_address);
>                  base_address <<= 4;
>                  break;
> -
> +            }
>              case 0x03:                  /* seg/off execution start address */
>              {
> -                u32 seg, off;
> +                long seg, off;
>
>                  seg = get2bytes(&p);
>                  off = get2bytes(&p);
> @@ -423,21 +421,16 @@ convert_S_records(
>      u8 cksum;
>      int incksum;
>      int c;
> -    int len;                        /* data length of current line */
> +    long len;                        /* data length of current line */
>      int rectype;                    /* record type */
> -    u32 addr;
> +    long addr;
>      bool endrecord = FALSE;
>      buffer_rec tb;
>
> -    while ( ! endrecord && (fgets(buff, sizeof(buff), ifp)))
> -    {
> +    while ( !endrecord && (fgets(buff, sizeof(buff), ifp)) ) {
>          p = &buff[0];
>
> -        if (p[strlen(p)-1] == '\n')                 /* get rid of newline */
> -            p[strlen(p)-1] = '\0';
> -
> -        if (p[strlen(p)-1] == '\r')                 /* get rid of any CR */
> -            p[strlen(p)-1] = '\0';
> +        fix_string_from_gets(p, sizeof(buff));
>
>          tb.dl_count = 0;
>
> @@ -452,8 +445,7 @@ convert_S_records(
>              badformat(buff, inm, BADLEN);
>          cksum = len;
>
> -        switch (rectype)
> -        {
> +        switch (rectype) {
>              case 0x00:                  /* comment field, ignored */
>                  goto write_it;
>
> @@ -476,9 +468,8 @@ convert_S_records(
>      doit:
>                  cksum += B0(addr) + B1(addr) + B2(addr) + B3(addr);
>
> -                tb.dl_destaddr = addr;
> -                while (len--)
> -                {
> +                tb.dl_destaddr = (u32) addr;
> +                while (len--) {
>                      if ((c = getbyte(&p)) == -1)
>                          badformat(buff, inm, BADDATA);
>                      cksum += c;
> @@ -543,33 +534,29 @@ convert_TI_records(
>      bool eol;
>      buffer_rec tb;
>
> -    while ( ! endrecord && (fgets(buff, sizeof(buff), ifp)))
> -    {
> +    while ( ! endrecord && (fgets(buff, sizeof(buff), ifp))) {
>          p = &buff[0];
> -        if (p[strlen(p)-1] == '\n')                 /* get rid of newline */
> -            p[strlen(p)-1] = '\0';
> -
> -        if (p[strlen(p)-1] == '\r')                 /* get rid of any CR */
> -            p[strlen(p)-1] = '\0';
> +        fix_string_from_gets(p, sizeof(buff));
>
>          tb.dl_count = 0;
>
>          eol = FALSE;
> -        while ( ! eol && ! endrecord)
> -        {
> -            switch (*p++)
> -            {
> +        while ( ! eol && ! endrecord) {
> +            switch (*p++) {
>                  case '9':
>                      if (tb.dl_count)
>                          write_record(&tb, ofp);
> -                    tb.dl_destaddr = get2bytes(&p);
> +                    tb.dl_destaddr = (u32) get2bytes(&p);
>                      break;
>
>                  case 'B':
>                      c = getbyte(&p);
> +                    assert( c != -1 );
>                      filesum += c;
>                      tb.dl_buf[tb.dl_count++] = c;
> +
>                      c = getbyte(&p);
> +                    assert( c != -1 );
>                      filesum += c;
>                      tb.dl_buf[tb.dl_count++] = c;
>                      break;
> @@ -596,8 +583,7 @@ void
>  write_record(buffer_rec *tb,
>               FILE *fp)
>  {
> -    if ( ! linear)
> -    {
> +    if ( ! linear) {
>          if (tb->dl_destaddr < base)
>              error(ERR_FATAL, "record at address 0x%x precedes base of 0x%x",
>                       tb->dl_destaddr, base);
> @@ -615,8 +601,7 @@ getnibble(char **p)
>      register int val;
>
>      **p = toupper(**p);
> -    switch (**p)
> -    {
> +    switch (**p) {
>          case '0': case '1': case '2': case '3': case '4':
>          case '5': case '6': case '7': case '8': case '9':
>              val = **p - '0';
> @@ -651,15 +636,14 @@ long
>  getNbytes(char **p,
>            int n)
>  {
> -    int t;
> +    long t;
>      u32 val = 0;
>
> -    while (n--)
> -    {
> +    while (n--) {
>          if ((t = getbyte(p)) == -1)
>              return(-1L);
>          val <<= 8;
> -        val += t;
> +        val += (u32) t;
>      }
>
>      return(val);
> @@ -670,8 +654,10 @@ badformat(char *s,
>            char *fname,
>            char *msg)
>  {
> -    if (s[strlen(s)-1] == '\n')             /* get rid of newline */
> +    if ( *s != '\0' ) {
> +      if (s[strlen(s)-1] == '\n')             /* get rid of newline */
>          s[strlen(s)-1] = '\0';
> +    }
>      error(0, "line '%s'::\n\tfrom file '%s'; %s", s, fname, msg);
>      exit(1);
>  }
> @@ -719,17 +705,31 @@ error(int error_flag, ...)
>
>      (void) fflush(stderr);
>
> -    if (error_flag & (ERR_FATAL | ERR_ABORT))
> -    {
> -        if (error_flag & ERR_FATAL)
> -        {
> +    if (error_flag & (ERR_FATAL | ERR_ABORT)) {
> +        if (error_flag & ERR_FATAL) {
>              error(0, "fatal error, exiting");
>              exit(local_errno ? local_errno : 1);
> -        }
> -        else
> -        {
> +        } else {
>              error(0, "fatal error, aborting");
>              abort();
>          }
>      }
>  }
> +
> +void  fix_string_from_gets(char *s, size_t max)
> +{
> +  size_t len;
> +  assert( s );
> +
> +  if ( *s == '\0' )
> +    return;
> +
> +  len = strnlen( s, max );
> +
> +  if ( s[len - 1] == '\n')                 /* get rid of newline */
> +      s[len - 1] = '\0';
> +
> +  if ( s[len - 1] == '\r')                 /* get rid of any CR */
> +      s[len - 1] = '\0';
> +
> +}
> --
> 1.9.3
>
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel



More information about the devel mailing list