Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > comp.lang.c > #386513
| From | Ben Bacarisse <ben@bsb.me.uk> |
|---|---|
| Newsgroups | comp.lang.c |
| Subject | Re: Fixing a sample from K&R book using cake static analyser |
| Date | 2024-06-25 14:36 +0100 |
| Organization | A noiseless patient Spider |
| Message-ID | <87pls5m9fb.fsf@bsb.me.uk> (permalink) |
| References | (2 earlier) <20240623034624.135@kylheku.com> <87wmmfq4if.fsf@bsb.me.uk> <20240624012527.8bbe16b96f5bfca10feadb5c@gmail.moc> <87zfrbnsvv.fsf@bsb.me.uk> <874j9h8l3d.fsf@fatphil.org> |
Phil Carmody <pc+usenet@asdf.org> writes:
> Ben Bacarisse <ben@bsb.me.uk> writes:
>> Anton Shepelev <anton.txt@gmail.moc> writes:
>>
>>> 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).
>>
>> With the usual trepidation that this is untested (though I did compile
>> it), I'd write something like:
>>
>> struct nlist *install(const char *name, const char *defn)
>> {
>> struct nlist *node = lookup(name);
>> if (node) {
>> char *new_defn = strdup(defn);
>> if (new_defn) {
>> free(node->defn);
>> node->defn = new_defn;
>> return node;
>> }
>> else return NULL;
>> }
>
> That's fine, certainly, but I'm a fan of early fail-and-bail, which
> permits the didn't-fail side of the if to remain unindented:
>
> if (node) {
> char *new_defn = strdup(defn);
> if (!new_defn)
> return NULL;
>
> free(node->defn);
> node->defn = new_defn;
> return node;
> }
I deliberately wanted to show that the fully structured "if then else"
way was perfectly clear and simple in this trivial function. Any
deviation from the easy-to-reason about version can then be assessed to
see if it's clearer? Does the fail-and-bail here help? I don't find
one simple indented block to be at all unclear so, to my mind, no it
doesn't.
To be fair, I write a lot of code like you've shown because it's become
a habit. I think there's a lot of it out there and it's easy enough to
follow so I often do it as well. But would I change one for the other?
No.
>> else {
>> struct nlist *new_node = malloc(sizeof *new_node);
>> char *new_name = strdup(name), *new_defn = strdup(defn);
>> if (new_node && new_name && new_defn) {
>> unsigned hashval = hash(name);
>> new_node->name = new_name;
>> new_node->defn = new_defn;
>> new_node->next = hashtab[hashval];
>> return hashtab[hashval] = new_node;
>> }
>> else {
>> free(new_defn);
>> free(new_name);
>> free(new_node);
>> return NULL;
>> }
>> }
>
> I think I'd deviate a little more on this side, as I don't like doing
> the strdups when you know the malloc has failed.
Why? If it were gathering other key resources, like locks, then I'd
agree, but can it really hurt much?
On the plus side, it gives all the variables free-able values -- either
NULL or a free-able pointer. That simplifies reasoning about any code
that follows.
> Again, early
> fail-and-bail applies:
>
> else {
> struct nlist *new_node = malloc(sizeof *new_node);
> if (!new_node)
> return NULL;
>
> char *new_name = strdup(name), *new_defn = strdup(defn);
But one strdup after another might have failed is OK? I see you address
this below...
> if (!new_name || !new_defn) {
> free(new_defn);
> free(new_name);
> free(new_node);
> return NULL;
> }
>
> unsigned hashval = hash(name);
> new_node->name = new_name;
> new_node->defn = new_defn;
> new_node->next = hashtab[hashval];
> return hashtab[hashval] = new_node;
> }
>
> You could split the strdups too, of course, but it's itsy-bitsy.
I don't get why you do one and not the other. Either it's all
itsy-bitsy or the sequencing is better and should always be preferred.
My main objection to fail-and-bail is a strong preference for expressing
the conditions for successful operation up front.
> In all
> honesty, I'd probably actually code with gotos, and jump to whichever
> bit of cleanup was relevant.
Why? I've yet to see a goto version of this that is even remotely as
clear as simply writing out what to do in the various cases.
>> We have four cases:
>>
>> node with the name found
>> new definition allocated
>> new definition not allocated
>> node with the name not found
>> new node, name and definition all allocated
>> not all of new node, name and definition allocated.
>
> I once came across a quirky coding standard that would have split
> this into 3 functions. The outermost if/else blocks would
> have been two separate helper functions.
That's not quirky; that's perfectly sound advice. I didn't do it here
because I wanted a direct comparison with all the spaghetti.
> struct nlist *install(const char *name, const char *defn)
> {
> struct nlist *node = lookup(name);
> if (node)
> return update_node(node, defn);
> else
> return new_node(name, defn);
> }
return node ? update_node(node, defn) : new_node(name, defn);
!
> The principle being that every function should do one job, and be
> visible in one small screenful. The choice of code structure in each of
> the two helper functions is now simplified, as you don't need to
> consider anything in the other helper function.
C slightly discourages this in that we need to add declarations or order
the functions "bottom-up". And, despite using static functions, it's
never clear where else the functions are being used because the smallest
restriction for use of a function is the file. That might not always
matter much but this example uses a global table, so it would be
important to check that. The end result is that C programmers tend to
use fewer auxiliary functions than programmers in some other languages.
--
Ben.
Back to comp.lang.c | Previous | Next — Previous in thread | Next in thread | Find similar
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
csiph-web