Re: [PATCH] pickles: Use EXCOND operator in {b,c}tf-dump.pk

[Mohammad-Reza Nabipoor]

Hi, David.

On Mon, Jan 03, 2022 at 11:30:23AM -0800, David Faust wrote:
> Hi Mohammad-Reza,
> 
> On 12/26/21 16:56, Mohammad-Reza Nabipoor wrote:
> > 2021-12-27  Mohammad-Reza Nabipoor  <mnabipoor@gnu.org>
> > 
> >     * pickles/btf-dump.pk (btf_dump_type_vdata): Use EXCOND operator.
> >     * pickles/ctf-dump.pk (ctf_dump_vlen_data): Likewise.
> > ---
> > 
> > Hi, David and Indu.
> > 
> > I didn't actually "run" this code, because I don't know much about BTF/CTF.
> > Could you please verify that this patch actually works?
> > (And it'd be nice if we have some tests for these pickles in
> > `testsuite/poke.pickles/`).
> 
> This is very nice, certainly much cleaner. Thanks for doing it :)
> 


We should thank Jose for this new syntax :)


> I tested both CTF and BTF (BTF more extensively since I have more laying
> around).
> 
> It works great with slight modification - since the individual dump
> functions are void, the calls need to be turned into statements by wrapping
> with { }, or else we fail with
> 
>   error: function doesn't return a value
> 
> i.e.
> +    ({ btf_dump_int (t.data.integer); } ?! E_elem)
> +      || ({ btf_dump_array (t.data.array); } ?! E_elem)
> ... etc.


Oh, right!
My bad!


> 
> Likewise for CTF. (Attached the edited hunks for convenience, please
> use/modify/ignore them as you like :) )
> 


Thank you very much!
I'll push them to master :)


> Otherwise LGTM, and thanks for the reminder of tests I will look into adding
> them.
> 

If BTF/CTF entities are small enough, you can embed data inside the
test file like this one: `testsuite/poke.pickles/id3v1-test.pk`.
Otherwise we can use pre-built binary files.

Thank you again!