[PATCH] top: fix double free causing a SIGABRT storm after SIGPIPE

[Rob Landley]

On 08/19/2016 01:30 PM, Denys Vlasenko wrote:
> On Fri, Aug 19, 2016 at 8:17 PM, Rob Landley <rob at landley.net> wrote:
>> On 08/19/2016 04:10 AM, Denys Vlasenko wrote:
>>> +       if (ENABLE_FEATURE_CLEAN_UP) {
>>> +               clearmems();
>>> +#if ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE
>>> +               free(prev_hist);
>>> +#endif
>>> +       }
>>
>> The point of the ENABLE macros was so that you can go:
>>
>>   if (ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE)
>>     free(prev_hist);
>>
>> Instead of the #ifdef forest. #ifdef CONFIG_FEATURE_BLAH already existed...
> 
> Indeed ifders are ugly.
> In this case, prev_hist does not exist if
> !ENABLE_FEATURE_TOP_CPU_USAGE_PERCENTAGE:
> 
> rocps/top.c:195:28: error: ‘struct globals’ has no member named ‘prev_hist’
>  #define prev_hist        (G.prev_hist         )
>                             ^
> procps/top.c:1258:9: note: in expansion of macro ‘prev_hist’
>     free(prev_hist);
>          ^~~~~~~~~

You're right, this entire file is an #ifdef forest. The top I wrote for
toybox (which lives in ps.c because ps, top, iotop, pgrep, and pkill
share code) doesn't have a single #if or #ifdef in it. Different design
priorities, I guess.

The kernel implemented this trick itself a few years back, using a
clever macro in include/linux/kconfig.h that does an
IS_ENABLED(CONFIG_BLAH) wrapper turning the ifdef/indef version into 1
or 0. So of course if you do this:

  $ grep -r '#if IS_ENABLED' linux | wc -l
  2555

Pet peeve of mine...

Rob