Groups > comp.lang.c > #386312 > unrolled thread

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

Started by	Thiago Adams <thiago.adams@gmail.com>
First post	2024-06-21 09:45 -0300
Last post	2024-06-29 00:19 +0000
Articles	20 on this page of 87 — 17 participants

Back to article view | Back to comp.lang.c

  Fixing a sample from K&R book using cake static analyser Thiago Adams <thiago.adams@gmail.com> - 2024-06-21 09:45 -0300
    Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-22 01:14 +0000
      Re: Fixing a sample from K&R book using cake static analyser Tim Rentsch <tr.17687@z991.linuxsc.com> - 2024-06-21 20:19 -0700
      Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@gmail.moc> - 2024-06-23 02:23 +0300
        Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-22 23:30 +0000
          Re: Fixing a sample from K&R book using cake static analyser bart <bc@freeuk.com> - 2024-06-23 00:53 +0100
            Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-23 23:50 +0000
          Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@gmail.moc> - 2024-06-24 01:59 +0300
            Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 00:31 +0000
        Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-23 11:23 +0100
          Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@gmail.moc> - 2024-06-24 01:33 +0300
            Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-24 00:36 +0100
              Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 00:29 +0000
              Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-24 04:38 +0200
                Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 02:56 +0000
              Re: Fixing a sample from K&R book using cake static analyser Tim Rentsch <tr.17687@z991.linuxsc.com> - 2024-06-24 02:21 -0700
      Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-23 11:02 +0000
        Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-23 12:31 +0100
          Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@gmail.moc> - 2024-06-24 01:25 +0300
            Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-23 22:58 +0000
              Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@gmail.moc> - 2024-06-24 02:14 +0300
                Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 00:31 +0000
            Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-24 00:25 +0100
              Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 00:34 +0000
                Re: Fixing a sample from K&R book using cake static analyser David Brown <david.brown@hesbynett.no> - 2024-06-24 11:39 +0200
                  Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 23:00 +0000
                    Re: Fixing a sample from K&R book using cake static analyser David Brown <david.brown@hesbynett.no> - 2024-06-25 10:25 +0200
                    Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-25 08:37 +0000
                Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-24 11:53 +0100
                  Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-24 23:01 +0000
                    Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-25 01:42 +0100
                    Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-25 01:21 +0000
                      Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-25 14:06 +0100
                        Re: Fixing a sample from K&R book using cake static analyser "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2024-06-25 20:35 -0700
                          Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-26 05:18 +0000
                            Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-26 10:56 +0200
                              Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-26 10:55 +0000
                                Re: Fixing a sample from K&R book using cake static analyser DFS <nospam@dfs.com> - 2024-06-26 07:20 -0400
                                [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-26 13:20 +0200
                                  Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Michael S <already5chosen@yahoo.com> - 2024-06-26 15:54 +0300
                                    Re: [OT] Reinheitsgebot and Beer without C Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-26 17:32 +0200
                                      Re: [OT] Reinheitsgebot and Beer without C Michael S <already5chosen@yahoo.com> - 2024-06-26 20:37 +0300
                                        Re: [OT] Reinheitsgebot and Beer without C Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-26 22:24 +0200
                                  Re: [OT] Re: Fixing a sample from K&R book using cake static analyser scott@slp53.sl.home (Scott Lurndal) - 2024-06-26 16:26 +0000
                                    Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Michael S <already5chosen@yahoo.com> - 2024-06-26 20:19 +0300
                                      Re: [OT] Re: Fixing a sample from K&R book using cake static analyser scott@slp53.sl.home (Scott Lurndal) - 2024-06-26 17:46 +0000
                                    Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-26 22:33 +0200
                                      Re: [OT] Re: Fixing a sample from K&R book using cake static analyser "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2024-06-26 14:09 -0700
                                        Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-26 23:41 +0200
                                          Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Richard Harnden <richard.nospam@gmail.invalid> - 2024-06-27 00:11 +0100
                                            Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-27 07:33 +0200
                                          Re: [OT] Re: Fixing a sample from K&R book using cake static analyser "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2024-06-28 15:46 -0700
                                            Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-29 02:20 +0200
                                              Re: [OT] Re: Fixing a sample from K&R book using cake static analyser gazelle@shell.xmission.com (Kenny McCormack) - 2024-06-29 11:26 +0000
                                                Re: [OT] Re: Fixing a sample from K&R book using cake static analyser "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2024-06-29 12:38 -0700
                                              Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Michael S <already5chosen@yahoo.com> - 2024-06-29 22:43 +0300
                                      Re: [OT] Re: Fixing a sample from K&R book using cake static analyser scott@slp53.sl.home (Scott Lurndal) - 2024-06-26 21:43 +0000
                                        Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-27 00:11 +0200
                                  Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Phil Carmody <pc+usenet@asdf.org> - 2024-06-28 13:42 +0300
                                    Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-28 14:04 +0200
                                      Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Michael S <already5chosen@yahoo.com> - 2024-06-28 17:11 +0300
                                        Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-28 19:12 +0200
                                          Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Tim Rentsch <tr.17687@z991.linuxsc.com> - 2024-06-28 11:07 -0700
                                            Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-28 20:18 +0200
                                              Re: [OT] Re: Fixing a sample from K&R book using cake static analyser Tim Rentsch <tr.17687@z991.linuxsc.com> - 2024-06-30 02:12 -0700
                                          Re: [OT] Re: Fixing a sample from K&R book using cake static analyser "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2024-06-28 15:56 -0700
                                            Re: [OT] Re: Fixing a sample from K&R book using cake static analyser "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2024-06-28 16:00 -0700
                                Re: Fixing a sample from K&R book using cake static analyser David Brown <david.brown@hesbynett.no> - 2024-06-26 13:24 +0200
                                  Re: Fixing a sample from K&R book using cake static analyser Richard Harnden <richard.nospam@gmail.invalid> - 2024-06-27 00:20 +0100
                                  Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-26 23:45 +0000
                                    Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-27 07:47 +0200
                              Re: Fixing a sample from K&R book using cake static analyser scott@slp53.sl.home (Scott Lurndal) - 2024-06-26 16:25 +0000
              Re: Fixing a sample from K&R book using cake static analyser Tim Rentsch <tr.17687@z991.linuxsc.com> - 2024-06-24 02:55 -0700
              Re: Fixing a sample from K&R book using cake static analyser Phil Carmody <pc+usenet@asdf.org> - 2024-06-25 11:47 +0300
                Re: Fixing a sample from K&R book using cake static analyser Ben Bacarisse <ben@bsb.me.uk> - 2024-06-25 14:36 +0100
                Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-25 22:51 +0000
                  Re: Fixing a sample from K&R book using cake static analyser Phil Carmody <pc+usenet@asdf.org> - 2024-06-30 23:33 +0300
          Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-23 22:36 +0000
        Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@gmail.moc> - 2024-06-24 01:40 +0300
          Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-23 23:04 +0000
            Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-24 01:31 +0000
              Re: Fixing a sample from K&R book using cake static analyser Anton Shepelev <anton.txt@g{oogle}mail.com> - 2024-06-24 14:28 +0300
            Re: Fixing a sample from K&R book using cake static analyser Janis Papanagnou <janis_papanagnou+ng@hotmail.com> - 2024-06-24 05:01 +0200
              Re: Fixing a sample from K&R book using cake static analyser Kaz Kylheku <643-408-1753@kylheku.com> - 2024-06-24 09:31 +0000
    Re: Fixing a sample from K&R book using cake static analyser Thiago Adams <thiago.adams@gmail.com> - 2024-06-22 17:32 -0300
    Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-29 00:02 +0000
      Re: Fixing a sample from K&R book using cake static analyser Lawrence D'Oliveiro <ldo@nz.invalid> - 2024-06-29 00:19 +0000

Page 1 of 5 [1] 2 3 4 5 Next page →

#386312 — Fixing a sample from K&R book using cake static analyser

From	Thiago Adams <thiago.adams@gmail.com>
Date	2024-06-21 09:45 -0300
Subject	Fixing a sample from K&R book using cake static analyser
Message-ID	<v53sl1$35qt7$1@dont-email.me>

In this video ( https://youtu.be/ZZCKPKzNUCQ
) I show step by step how removing warnings will fix a memory leak.


Page 145, The C programming Language 2 Edition

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

     if ((np = lookup(name)) == NULL) {  /* not found */
         np = (struct nlist *) malloc(sizeof(*np));
         if (np == NULL || (np->name = strdup(name)) == NULL)
             return NULL;
         hashval = hash(name);
         np->next = hashtab[hashval];
         hashtab[hashval] = np;
     } else      /* already there */
         free((void *) np->defn);  /* free previous defn */

     if ((np->defn = strdup(defn)) == NULL)
         return NULL;
     return np;
}

The concepts used are

  - ownership transfer
  - nullable pointers

Concepts are described here

http://thradams.com/cake/ownership.html

To see the sample and play with it:

http://thradams.com/cake/playground.html

Select "find the bug" and "bug #7 K & R"

[toc] | [next] | [standalone]

#386338

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-22 01:14 +0000
Message-ID	<v558hv$3dskb$1@dont-email.me>
In reply to	#386312

On Fri, 21 Jun 2024 09:45:21 -0300, Thiago Adams wrote:

> Page 145, The C programming Language 2 Edition
> 
> /* install:  put (name, defn) in hashtab */
> struct nlist *install(char *name, char *defn)
> {
>      struct nlist *np;
>      unsigned hashval;
> 
>      if ((np = lookup(name)) == NULL) {  /* not found */
>          np = (struct nlist *) malloc(sizeof(*np));
>          if (np == NULL || (np->name = strdup(name)) == NULL)
>              return NULL;
>          hashval = hash(name);
>          np->next = hashtab[hashval];
>          hashtab[hashval] = np;
>      } else      /* already there */
>          free((void *) np->defn);  /* free previous defn */
> 
>      if ((np->defn = strdup(defn)) == NULL)
>          return NULL;
>      return np;
> }

    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*/

[toc] | [prev] | [next] | [standalone]

#386339

From	Tim Rentsch <tr.17687@z991.linuxsc.com>
Date	2024-06-21 20:19 -0700
Message-ID	<86le2xhdfa.fsf@linuxsc.com>
In reply to	#386338

Lawrence D'Oliveiro <ldo@nz.invalid> writes:

> On Fri, 21 Jun 2024 09:45:21 -0300, Thiago Adams wrote:
>
>> Page 145, The C programming Language 2 Edition
>>
>> /* install:  put (name, defn) in hashtab */
>> struct nlist *install(char *name, char *defn)
>> {
>>      struct nlist *np;
>>      unsigned hashval;
>>
>>      if ((np = lookup(name)) == NULL) {  /* not found */
>>          np = (struct nlist *) malloc(sizeof(*np));
>>          if (np == NULL || (np->name = strdup(name)) == NULL)
>>              return NULL;
>>          hashval = hash(name);
>>          np->next = hashtab[hashval];
>>          hashtab[hashval] = np;
>>      } else      /* already there */
>>          free((void *) np->defn);  /* free previous defn */
>>
>>      if ((np->defn = strdup(defn)) == NULL)
>>          return NULL;
>>      return np;
>> }
>
>     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*/

Both of these are truly awful.

[toc] | [prev] | [next] | [standalone]

#386366

From	Anton Shepelev <anton.txt@gmail.moc>
Date	2024-06-23 02:23 +0300
Message-ID	<20240623022343.ec355c69a3d9eb03ad4a50f2@gmail.moc>
In reply to	#386338

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

[toc] | [prev] | [next] | [standalone]

#386367

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-22 23:30 +0000
Message-ID	<v57mqf$3vq0p$5@dont-email.me>
In reply to	#386366

On Sun, 23 Jun 2024 02:23:43 +0300, Anton Shepelev wrote:

> Why are you so afraid of `goto' ...

Look up the concept of a “Nassi-Shneiderman diagram”. It allows arbitrary 
nesting of complex code, with dynamic allocation going on, while 
minimizing flow-control headaches. The example I posted was a simple one; 
I have more complex ones if you want to see.

> I think you forget to set np->name (and to free() it in
> case of error).

Ah, didn’t notice that, since it was hidden in the middle of another line 
of code. The fix is simple. And while I’m at it, it makes sense to factor 
out the table entry disposal code into a separate routine.

    void np_free(struct nlist *np)
      {
        if (np != NULL)
          {
            free(np->name);
            free(np->defn);
          } /*if*/
        free(np);
      } /*np_free*/

    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->name = strdup(name);
            if (np->name == 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);
        np_free(np);
        return
            result;
      } /*install*/

[toc] | [prev] | [next] | [standalone]

#386369

From	bart <bc@freeuk.com>
Date	2024-06-23 00:53 +0100
Message-ID	<v57o51$3vj82$1@dont-email.me>
In reply to	#386367

On 23/06/2024 00:30, Lawrence D'Oliveiro wrote:
> On Sun, 23 Jun 2024 02:23:43 +0300, Anton Shepelev wrote:
> 
>> Why are you so afraid of `goto' ...
> 
> Look up the concept of a “Nassi-Shneiderman diagram”. It allows arbitrary
> nesting of complex code, with dynamic allocation going on, while
> minimizing flow-control headaches. The example I posted was a simple one;
> I have more complex ones if you want to see.

Plus of course Python doesn't have 'goto' otherwise there would be no 
need of it.

[toc] | [prev] | [next] | [standalone]

#386418

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-23 23:50 +0000
Message-ID	<v5acc8$imk3$1@dont-email.me>
In reply to	#386369

On Sun, 23 Jun 2024 00:53:04 +0100, bart wrote:

> On 23/06/2024 00:30, Lawrence D'Oliveiro wrote:
>
>> On Sun, 23 Jun 2024 02:23:43 +0300, Anton Shepelev wrote:
>> 
>>> Why are you so afraid of `goto' ...
>> 
>> Look up the concept of a “Nassi-Shneiderman diagram”. It allows
>> arbitrary nesting of complex code, with dynamic allocation going on,
>> while minimizing flow-control headaches.

Another point is idempotency of disposal routines. By which I mean that 
attempting to free a NULL pointer is a harmless no-op. How many times have 
you seen pointless rigmarole like

    if (p1)
        free(p1);
    if (p2)
        free(p2);
    if (p3)
        free(p3);

when it could be written so much more simply as

    free(p1);
    free(p2);
    free(p3);

You’ll notice that my np_free routine can be used in the same way.

[toc] | [prev] | [next] | [standalone]

#386410

From	Anton Shepelev <anton.txt@gmail.moc>
Date	2024-06-24 01:59 +0300
Message-ID	<20240624015940.be2546d45cc185a330bdee55@gmail.moc>
In reply to	#386367

Lawrence D'Oliveiro to Anton Shepelev:

> > Why are you so afraid of `goto' ...
>
> Look up the concept of a "Nassi-Shneiderman diagram". It
> allows arbitrary nesting of complex code, with dynamic
> allocation going on, while minimizing flow-control
> headaches.

Thank you, I will:

   http://www.cs.umd.edu/hcil/members/bshneiderman/nsd/1973.pdf

I hate the traditional flowcharts, and the N.-S. certainly
look so much better.

> And while I'm at it, it makes sense to factor out the
> table entry disposal code into a separate routine.
>
>     void np_free(struct nlist *np)
>       {
>         if (np != NULL)
>           {
>             free(np->name);
>             free(np->defn);
>           } /*if*/
>         free(np);
>       } /*np_free*/

I thought the challenge was to fix it as a single function.
np_free() helps, but is a tad redundant in that it always
tried to dispose of the whole thing, even when at calling
point we know for certain not all three object have been
allocted.  You further simplify things by zero-filling via
calloc() and relying on free() accepting NULL pointers,
whereas I prefere to avoid such redundant calls of free().

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

[toc] | [prev] | [next] | [standalone]

#386422

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-24 00:31 +0000
Message-ID	<v5aeot$j1nj$2@dont-email.me>
In reply to	#386410

On Mon, 24 Jun 2024 01:59:40 +0300, Anton Shepelev wrote:

> I thought the challenge was to fix it as a single function.

I don’t know of any “challenge”, I just saw some code that could be 
improved, and so I improved it.

I am pretty sure that disposal of table entries would be needed as a 
separate function in a more complete lookup-table implementation, which is 
why I factored it out at this point.

[toc] | [prev] | [next] | [standalone]

#386377

From	Ben Bacarisse <ben@bsb.me.uk>
Date	2024-06-23 11:23 +0100
Message-ID	<878qywq7ou.fsf@bsb.me.uk>
In reply to	#386366

Anton Shepelev <anton.txt@gmail.moc> writes:

> 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'?

Why are you so fond of them that you add three extra state variables
that have to be (mentally and/or mathematically tracked) just understand
this supposedly simply function?

> 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;
> }

This, to me, is a textbook case of why goto is harmful.  I have spend
considerable time jumping up and down checking the state of all the
variables and I am still not sure I follow what this supposedly simple
function is doing.  I am pretty sure it is not functionally the same as
the original, but it's a struggle to follow it.

(And everyone seems keen on redundant casts.  Why?)

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#386406

From	Anton Shepelev <anton.txt@gmail.moc>
Date	2024-06-24 01:33 +0300
Message-ID	<20240624013337.36fdb40f0766c6e1c8ce67c7@gmail.moc>
In reply to	#386377

Ben Bacarisse to Anton Shelelev:

> > Why are you so afraid of `goto' that you must emulate it
> > with `while' and `break'?
>
> Why are you so fond of them that you add three extra state
> variables that have to be (mentally and/or mathematically
> tracked) just understand this supposedly simply function?

Because my attempts to fix the function without the extra
variables proved even worse.

> This, to me, is a textbook case of why goto is harmful.  I
> have spend considerable time jumping up and down checking
> the state of all the variables and I am still not sure I
> follow what this supposedly simple function is doing.

I have tried to keep the structure simple: all the goto's
jump down, both the labels define isolated blocks, guared by
returns, to prevent fall-though.

> (And everyone seems keen on redundant casts.  Why?)

As I said, I could not test the function without extra work,
so I tried to change as little as possible.

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

[toc] | [prev] | [next] | [standalone]

#386416

From	Ben Bacarisse <ben@bsb.me.uk>
Date	2024-06-24 00:36 +0100
Message-ID	<87tthjnsdt.fsf@bsb.me.uk>
In reply to	#386406

Anton Shepelev <anton.txt@gmail.moc> writes:

> Ben Bacarisse to Anton Shelelev:
>
>> > Why are you so afraid of `goto' that you must emulate it
>> > with `while' and `break'?
>>
>> Why are you so fond of them that you add three extra state
>> variables that have to be (mentally and/or mathematically
>> tracked) just understand this supposedly simply function?
>
> Because my attempts to fix the function without the extra
> variables proved even worse.

OK, but two labels and three extra state variables makes it much more
complex to think about.  I don't think the simple "if this then that"
approach is at all problematic in this case.  I can't see why adding
even one goto helps when the structured approach is so simple.

(I've tried to show this with code in another reply.)

>> This, to me, is a textbook case of why goto is harmful.  I
>> have spend considerable time jumping up and down checking
>> the state of all the variables and I am still not sure I
>> follow what this supposedly simple function is doing.
>
> I have tried to keep the structure simple: all the goto's
> jump down, both the labels define isolated blocks, guared by
> returns, to prevent fall-though.

Trying to make gotos less bad than they can be is not usually an overall
positive.  Whenever I see a "good" use of goto, I try to see if there's
a better way with no use of goto.  So far (with the exception of an
example of tightly bound co-routines being simulated in a single C
function) I have not yet seen one that can't.  There are sometimes
run-time considerations (giant state machines, for example) but even
there the structured code is probably clearer.  The use of gotos in
those cases is not to improve the logic of the code but to placate the
code generator.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#386421

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-24 00:29 +0000
Message-ID	<v5aem2$j1nj$1@dont-email.me>
In reply to	#386416

On Mon, 24 Jun 2024 00:36:46 +0100, Ben Bacarisse wrote:

> So far (with the exception of an example of tightly bound co-routines
> being simulated in a single C function) I have not yet seen one that
> can't [be done without goto].

Wouldn’t it be cool if C had continuations?

I succeeded in implementing them in a PostScript-alike toy language I’ve 
been messing with (see my postings on comp.lang.postscript if you want to 
know more), and they weren’t that hard to do at all.

Making proper use of them is another matter.

[toc] | [prev] | [next] | [standalone]

#386431

From	Janis Papanagnou <janis_papanagnou+ng@hotmail.com>
Date	2024-06-24 04:38 +0200
Message-ID	<v5am80$o3os$1@dont-email.me>
In reply to	#386416

On 24.06.2024 01:36, Ben Bacarisse wrote:
>> [...]
> 
> Trying to make gotos less bad than they can be is not usually an overall
> positive.  Whenever I see a "good" use of goto, I try to see if there's
> a better way with no use of goto. 

This is the same impetus I have.

> So far (with the exception of an
> example of tightly bound co-routines being simulated in a single C
> function) I have not yet seen one that can't.  There are sometimes
> run-time considerations (giant state machines, for example) but even
> there the structured code is probably clearer.  The use of gotos in
> those cases is not to improve the logic of the code but to placate the
> code generator.

I recall from past decades that I've seen only one specific code
pattern where I'd say that 'goto' would not make a program worse
(or even make it simpler); it is a (more or less) deeply nested
imperative code construct of loops where we want to leave from
the innermost loop. Propagating the exit condition through all
the nested loops "to the surface" complicates the code here. The
introduction of additional state or duplication of code is what
the literature (dating back to "goto considered harmful" times)
gives as (sole) rationale for its use.

It still has an inherent "bad smell" since you can circumvent a
lot code (even leaving stack contexts). Some languages have thus
introduced yet more restricted forms; e.g. the 'break N' in the
Unix standard shell language, allowing one to leave N levels of
nested (for/while/until) loops.

Janis

[toc] | [prev] | [next] | [standalone]

#386432

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-24 02:56 +0000
Message-ID	<v5an9c$o87b$1@dont-email.me>
In reply to	#386431

On Mon, 24 Jun 2024 04:38:54 +0200, Janis Papanagnou wrote:

> ... it is a (more or less) deeply nested imperative code
> construct of loops where we want to leave from the innermost loop.
> Propagating the exit condition through all the nested loops "to the
> surface" complicates the code here.

Maybe it’s the kind of code I write, but I typically need to do cleanups 
on the way out of such nested constructs. That means a goto won’t cut it.

[toc] | [prev] | [next] | [standalone]

#386443

From	Tim Rentsch <tr.17687@z991.linuxsc.com>
Date	2024-06-24 02:21 -0700
Message-ID	<86ed8mg0hn.fsf@linuxsc.com>
In reply to	#386416

Ben Bacarisse <ben@bsb.me.uk> writes:

[...]

> Trying to make gotos less bad than they can be is not usually an
> overall positive.  Whenever I see a "good" use of goto, I try to
> see if there's a better way with no use of goto.  So far [noted an
> exception] I have not yet seen one that can't.

Some years ago I was revising/rewriting some code, and got to a
point where using a goto seemed like the best way to proceed.
And it was a really awful goto too, the kind of horror show one
might see in an obfuscated C contest.  The argument in favor of
using the goto is that not using it would have meant a really
ugly duplication of part of the algorithm.  There is no question
that I could have avoided using goto, but in that particular case
using goto seemed like a better choice than the alternatives.

Generally speaking I share your reaction to using goto.
Sometimes though using goto gives a nicer looking result
than local-scale alternatives.

[toc] | [prev] | [next] | [standalone]

#386378

From	Kaz Kylheku <643-408-1753@kylheku.com>
Date	2024-06-23 11:02 +0000
Message-ID	<20240623034624.135@kylheku.com>
In reply to	#386338

On 2024-06-22, Lawrence D'Oliveiro <ldo@nz.invalid> wrote:
> On Fri, 21 Jun 2024 09:45:21 -0300, Thiago Adams wrote:
>
>> Page 145, The C programming Language 2 Edition
>> 
>> /* install:  put (name, defn) in hashtab */
>> struct nlist *install(char *name, char *defn)
>> {
>>      struct nlist *np;
>>      unsigned hashval;
>> 
>>      if ((np = lookup(name)) == NULL) {  /* not found */
>>          np = (struct nlist *) malloc(sizeof(*np));
>>          if (np == NULL || (np->name = strdup(name)) == NULL)
>>              return NULL;
>>          hashval = hash(name);
>>          np->next = hashtab[hashval];
>>          hashtab[hashval] = np;
>>      } else      /* already there */
>>          free((void *) np->defn);  /* free previous defn */
>> 
>>      if ((np->defn = strdup(defn)) == NULL)
>>          return NULL;
>>      return np;
>> }
>
>     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);

The cast is not needed in C; only if you need to compile this
as C++.  The type variant of sizeof requires parentheses:

 sizeof expr
 sizeof (type)

>             if (np == NULL)
>                 break;
>             np->defn = strdup(defn);

What happening to duplicating the name into np->name?

>             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*/

What scatter-brained drivel. Watch and learn:

  struct nlist *install(char *name, char *defn)
  {
    struct nlist *existing = lookup(name);

    if (existing) {
      return existing;
    } else {
      struct nlist *np = calloc(1, sizeof (struct nlist));
      char *dupname = strdup(name);
      char *dupdefn = strdup(defn);
      unsigned hashval = hash(name);

      if (np && dupname && dupdefn) {
        np->name = dupname;
        np->defn = dupdefn;
        np->next = hashtab[hashval];
        hashtab[hashval] = np;
        return np;
      }

      free(dupdefn);
      free(dupname);
      free(np);

      return NULL;
    }
  }


-- 
TXR Programming Language: http://nongnu.org/txr
Cygnal: Cygwin Native Application Library: http://kylheku.com/cygnal
Mastodon: @Kazinator@mstdn.ca

[toc] | [prev] | [next] | [standalone]

#386380

From	Ben Bacarisse <ben@bsb.me.uk>
Date	2024-06-23 12:31 +0100
Message-ID	<87wmmfq4if.fsf@bsb.me.uk>
In reply to	#386378

Kaz Kylheku <643-408-1753@kylheku.com> writes:

>> On Fri, 21 Jun 2024 09:45:21 -0300, Thiago Adams wrote:
>>
>>> Page 145, The C programming Language 2 Edition
>>> 
>>> /* install:  put (name, defn) in hashtab */
>>> struct nlist *install(char *name, char *defn)
>>> {
>>>      struct nlist *np;
>>>      unsigned hashval;
>>> 
>>>      if ((np = lookup(name)) == NULL) {  /* not found */
>>>          np = (struct nlist *) malloc(sizeof(*np));
>>>          if (np == NULL || (np->name = strdup(name)) == NULL)
>>>              return NULL;
>>>          hashval = hash(name);
>>>          np->next = hashtab[hashval];
>>>          hashtab[hashval] = np;
>>>      } else      /* already there */
>>>          free((void *) np->defn);  /* free previous defn */
>>> 
>>>      if ((np->defn = strdup(defn)) == NULL)
>>>          return NULL;
>>>      return np;
>>> }

[snip attempts at tidying up...]
> Watch and learn:
>
>   struct nlist *install(char *name, char *defn)
>   {
>     struct nlist *existing = lookup(name);
>
>     if (existing) {
>       return existing;
>     } else {
>       struct nlist *np = calloc(1, sizeof (struct nlist));
>       char *dupname = strdup(name);
>       char *dupdefn = strdup(defn);
>       unsigned hashval = hash(name);
>
>       if (np && dupname && dupdefn) {
>         np->name = dupname;
>         np->defn = dupdefn;
>         np->next = hashtab[hashval];
>         hashtab[hashval] = np;
>         return np;
>       }
>
>       free(dupdefn);
>       free(dupname);
>       free(np);
>
>       return NULL;
>     }
>   }

You've over-simplified.  The function needs to replace the definition
with a strdup'd string (introduction another way to fail) when the name
is found by lookup.  It's just another nested if that's needed.  I don't
get why the goto crowd want to complicate it so much.

-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#386403

From	Anton Shepelev <anton.txt@gmail.moc>
Date	2024-06-24 01:25 +0300
Message-ID	<20240624012527.8bbe16b96f5bfca10feadb5c@gmail.moc>
In reply to	#386380

Ben Bacarisse:

> I don't get why the goto crowd want to complicate it so
> much.

Will someone post a goto-less that fixes what I perceive as
bugs in the original:

  a.  the failure to free() a newly allocated nlist in case
      of a later error,

  b.  the failure to free() a newly allocated np->name in
      case of a later error,

  c.  the failure to keep np->defn unchaged if the
      allocation of the new defn value failed.

And my perception be wrong, let us first establish the
actual bug(s).

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

[toc] | [prev] | [next] | [standalone]

#386409

From	Lawrence D'Oliveiro <ldo@nz.invalid>
Date	2024-06-23 22:58 +0000
Message-ID	<v5a99v$i4c8$2@dont-email.me>
In reply to	#386403

On Mon, 24 Jun 2024 01:25:27 +0300, Anton Shepelev wrote:

> Will someone post a goto-less that fixes what I perceive as bugs in the
> original:
> 
>   a.  the failure to free() a newly allocated nlist in case
>       of a later error,
> 
>   b.  the failure to free() a newly allocated np->name in
>       case of a later error,

All done.

>   c.  the failure to keep np->defn unchaged if the
>       allocation of the new defn value failed.

Not sure what the point is here.

[toc] | [prev] | [next] | [standalone]

Page 1 of 5 [1] 2 3 4 5 Next page →

csiph-web

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

Contents

#386312 — Fixing a sample from K&R book using cake static analyser

#386338

#386339

#386366

#386367

#386369

#386418

#386410

#386422

#386377

#386406

#386416

#386421

#386431

#386432

#386443

#386378

#386380

#386403

#386409