Path: ...!news.mixmin.net!eternal-september.org!feeder3.eternal-september.org!news.eternal-september.org!.POSTED!not-for-mail
From: Anton Shepelev <anton.txt@gmail.moc>
Newsgroups: comp.lang.c
Subject: Re: Fixing a sample from K&R book using cake static analyser
Date: Sun, 23 Jun 2024 02:23:43 +0300
Organization: A noiseless patient Spider
Lines: 79
Message-ID: <20240623022343.ec355c69a3d9eb03ad4a50f2@gmail.moc>
References: <v53sl1$35qt7$1@dont-email.me>
	<v558hv$3dskb$1@dont-email.me>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Injection-Date: Sun, 23 Jun 2024 01:23:44 +0200 (CEST)
Injection-Info: dont-email.me; posting-host="c3c9037eb3548b249d90960eb3690377";
	logging-data="4190541"; mail-complaints-to="abuse@eternal-september.org";	posting-account="U2FsdGVkX1/28ZtDomrbnVXhIdx7RZeWpgRwieFeRUg="
Cancel-Lock: sha1:hO25QZZLNpE84kFDIFeP2XEEaco=
X-Newsreader: Sylpheed 3.7.0 (GTK+ 2.24.30; i686-pc-mingw32)
Bytes: 3394

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