Re: Fixing a sample from K&R book using cake static analyser

Lawrence D'Oliveiro:

    struct nlist *install(char *name, char *defn)
      {
        struct nlist *np = NULL;
        struct nlist *result = NULL;
        unsigned hashval;
        do /*once*/
          {
            result = lookup(name);
            if (result != NULL)
                break;
            np = (struct nlist *)calloc(1, sizeof struct nlist);
            if (np == NULL)
                break;
            np->defn = strdup(defn);
            if (np->defn == NULL)
                break;
            hashval = hash(name);
            np->next = hashtab[hashval];
            hashtab[hashval] = np;
            result = np;
            np = NULL; /* so I don't dispose of it yet */
          }
        while (false);
        if (np != NULL)
          {
            free(np->defn);
          } /*if*/
        free(np);
        return
            result;
      } /*install*/

Why are you so afraid of `goto' that you must emulate it
with `while' and `break'?  I think you forget to set
np->name (and to free() it in case of error).  You set
np->defn only in case of a new node, whereas the original
code did it for an existing node as well.  My absolutely
untested version is below:

/* install:  put (name, defn) in hashtab */
struct nlist *install(char *name, char *defn)
{  struct   nlist *np;
   unsigned        hashval;
   int             new_nm, new_nd;
   void*           old_fn;

   new_nm = 0; new_nd = 0; old_fn = NULL;

   if ((np = lookup(name)) != NULL) /* short branch first     */
   {  old_fn = (void *)np->defn;    /* do not free it here to */
      goto set_defn;  }             /* avoid a side effect    */

   np = (struct nlist *) malloc(sizeof(*np));
   if(np == NULL) goto error;
   new_nd = 1;

   if((np->name = strdup(name)) == NULL) goto error;
   new_nm = 1;

   hashval          = hash(name);
   np->next         = hashtab[hashval];
   hashtab[hashval] = np;

set_defn:
   if ((np->defn = strdup(defn)) == NULL) goto error;

   if( old_fn ) free( old_fn );
   return np;
error:
   if( new_nm ) free( np->name );
   if( new_nd ) free( np       );
   return NULL;
}

--
()  ascii ribbon campaign -- against html e-mail
/\  www.asciiribbon.org   -- against proprietary attachments