Re: [PATCH 05/14] segment_report_module: Pull finish_portion() info file scope

[Mark Wielaard <mark@klomp.org>]

Hi Timm,

On Thu, 2020-11-12 at 16:04 +0100, Timm Bäder via Elfutils-devel wrote:
> +static inline void
> +finish_portion (Dwfl *dwfl,
> +                Dwfl_Memory_Callback *memory_callback,
> +                void *memory_callback_arg,
> +                void **data, size_t *data_size)
> +{
> +  if (*data_size != 0 && *data != NULL)
> +    (void) segment_read (dwfl, memory_callback, memory_callback_arg,
> +                         -1, data, data_size, 0, 0);
> +}

And again I would like to see this simply inlined instead of
introducing a top-level function that just takes some arguments that it
then rearranges to call a different function that again rearranges its
arguments to call one of the arguments.

I admit this messes up the rest of the patch series, so that none of it
applies cleanly anymore. Apologies.

I do like your creation of single loops, so you can inline some of the
inner functions directly in the loop instead of having multiple
callers. I think all those look good.

The only two commits I have to think about are:
[06/14] segment_report_module: Pull read_portion() into file scope
[07/14] segment_report_module: Use a struct for build id information

Which feel a little clunky to me. But I can probably be convinced they
are OK improvements if you simply say you disagree and resubmit them as
is.

Thanks,

Mark