On 05-03-19 21:32, Tom de Vries wrote:
> On 04-03-19 18:09, Jakub Jelinek wrote:
>> On Mon, Mar 04, 2019 at 05:49:26PM +0100, Tom de Vries wrote:
>>> 2019-03-04  Tom de Vries  <tdevries@suse.de>
>>>
>>> 	PR dwz/24301
>>> 	* dwz.c (main): Handle dwz returning 1 if processing more than one file.
>>> 	* testsuite/dwz.tests/two-files-too-many-dies.sh: New test.
>>
>> That looks wrong to me, that means once an error is reported for any single
>> one, the rest will not be processed.  That was not the intent and is
>> undesirable for the distro compression, we want to compress as much as
>> possible.
>>
> 
> I see, understood. Updated the test-case accordingly.
> 
>> Guess that
>>           else if (resa[i - optind].res == 0)
>>             successcount++;
>> should be
>>           if (resa[i - optind].res == 0)
>>             successcount++;
>> 	  else
>> 	    ret = 1;
>> or something similar, I must say I don't really remember the difference
>> between res->res value and return value from dwz etc.,
> 
> The return value of the dwz function indicates whether a problem
> occurred during dwz execution.
> 
> The res->res value, as far as I can infer from the source, is related to
> file status, and can be (as added in the patch for PR24275):
> ...
>   /* -2: Already processed under different name.
>      -1: Ignore.
>       0: Processed, changed.
>       1: Processed, unchanged.
> ...
> and unchanged can be either due to an error, or due to not being able to
> compress.
> 
> [ The PR24275 is about that fact that dwz never in fact returns with
> res->res set to 1. ]
> 
>> but it seems weird
>> that if if some file isn't processed in normal mode and when switched into lowmem
>> mode succeeds, we don't count it as successcount,
> 
> The successcount variable tracks the amount of unique files (that is, in
> hardlink mode we count two hardlinks to the same file as one) that
> succeeded in regular mode, which, if >=2 enables multifile mode.
> 
> AFAIU, the fact that low-mem files are not optimized in multifile mode
> is listed explicitly in the file header documentation:
> ...
>    If some executable or shared library has too large debug information
> 
>    (number of DIEs in .debug_info section) that there would be
> 
>    risk of too high memory consumption, that file isn't multifile
> 
>    optimized, instead it is processed by dwz () in a low-memory mode
> 
>    with low_mem flag set.  This can decrease memory consumption to
> 
>    half in some very large cases.  */
> ...
> 
> So, I'd say we want to keep the else in:
> ...
>            else if (resa[i - optind].res == 0)
> ...
> 
>> while if we already are in
>> lowmem mode from the previous file, we count it.
> 
> The lowmem mode is a per-file mode. If a previous file was processed in
> low-mem mode, we still try the current file again in regular mode,
> before possibly switching back to low-mem mode.
> 
> ---
> 
> Also, we cannot use and else here:
> ...
>  	  else
>  	    ret = 1;
> ...
> because that also triggers for res->res == -2 (which would make
> hardlink.sh fail). So I use:
> ...
>          else if (thisret == 1)
> ...
> instead.
> 
> OK like this?

Committed as attached.

Thanks,
- Tom