[Toybox] [PATCH] gpiod: fix foreach_chip crashes.

[Rob Landley]

On 6/12/23 09:32, enh wrote:> On Sat, Jun 10, 2023 at 1:41 PM Rob Landley
<rob at landley.net
> <mailto:rob at landley.net>> wrote:
> 
>     On 6/10/23 00:05, enh via Toybox wrote:
>     > This was segfaulting because the loop copying the chips into the array
>     > was corrupting the list, and because the llist_traverse was being given
>     > the wrong pointer.
> 
>     Huh, I wonder how that segfaults. I still don't have a test environment for
>     this, but foreach_chip() is called exactly once (from three different command
>     main functions) so consuming the list in the one and only pass seemed
>     reasonable? I see I forgot to remove the free before exit but the only way there
>     should be anything left is if TT.chip_count was < than the number of entries in
>     the list, and even then the remains of the list should still be in their
>     original order...?
> 
> oh, yeah, just not trying to free the now-corrupted list would have been fine
> too. (really there were two unrelated bugs: the traversal meant that there was a
> leak, and the free was using the wrong pointer, which was the segfault. just
> fixing the segfault didn't make any sense on its own.)
>  
>     *shrug* Applied your patch as-is, I shouldn't poke at code I can't test...
> 
> i can send you a tested patch for not freeing if you prefer.

Six of one... I was just saying "when I changed this bit I forgot to change that
bit".

In theory making the free() optional reduces the size of the binary, and various
embedded environments still use brk() under the covers, meaning to free the
entire heap at exit() they just have to move one pointer. A lot of "what can we
do in 64k of ram" embedded environments look back at what Unix v6 and friends
_did_ in similarly sized environments, and it still works. If you want a posix
programming environment in tiny RTOS, what's old is often new again. And yes, we
have reached the "on-chip battery" stage of embedded chicanery:

https://www.independent.co.uk/tech/battery-smallest-computer-wearables-b2021494.html

>     > The strdup wasn't failing, but should be an xstrdup anyway. (I think
>     > all the strdups in toys/ should actually be xstrdups. Maybe we should
>     > even poison strdup, or just `#define strdup xstrdup`?)
> 
>     I've historically done some poisoning, but it tended to cause weird build breaks
>     with macros in header files and so on? 
> 
> 
> there's a #pragma `poison` in both gcc and clang (specifically for this
> problem). as long as we're building with those compilers, that should prevent
> any uses creeping in?

Is there a list of clang pragmas anywhere? The llvm documentation at
https://clang.llvm.org/docs/UsersManual.html does not seem to have one...

Rob