[Swan-dev] why I added a consistency check to pbs_in_struct
andrew.cagney at gmail.com
Sat May 8 17:50:43 UTC 2021
On Sat, 8 May 2021 at 12:27, D. Hugh Redelmeier <hugh at mimosa.com> wrote:
> | commit 9945236619b17fa13dfd1cfbe60359dcbf3fcd21
> | Author: D. Hugh Redelmeier <hugh at mimosa.com>
> | Date: Fri May 7 23:58:52 2021 -0400
> | pluto: packet.c: add consistency check to pbs_in_struct
> | pbs_in_struct now requires that if and only if the structure has a
> | length field then the obj_pbs isn't NULL.
> | There are a few places where this part was intentionally ignored.
> | This required adding a dummy pbs variable ("ignored").
> Structs, as understood by packet.h, sometimes have variable parts
> after the fixed fields.
> Here's an extract from the ancient comments at the head of
> pbs_in_struct. The last parameter of pbs_in_struct is
> struct pbs_in *obj_pbs"
> The phrase "is supplied" means "is not NULL".
The old behavior was that a NULL implied that the nested struct, if any,
could be ignored. I'd call that obvious behaviour.
(passing a pbs when not needed was benign).
Now, if this is done wrong, and on a rarely executed code path, we've got a
ticking time bomb.
It's no longer obvious from a simple visual check of the call that passing
in NULL or not isn't going to explode.
> * If obj_pbs is supplied, a new pb_stream is created for the
> * variable part of the structure (this depends on their
> * being one length field in the structure). The cursor of this
> * new PBS is set to after the parsed part of the struct.
> I was prompted to add this because it would have caught a mistake we
> made but never committed. It has not caught any actual bugs in our
> code. The more checking, the more confidence we can have in our code.
Would it have crashed pluto?
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Swan-dev