On 06/26/2017 09:26 AM, Jakub Jelinek wrote:
> On Mon, Jun 26, 2017 at 09:22:31AM -0600, Jeff Law wrote:
>>>>> From d255827a64012fb81937d6baa8534eabecf9b735 Mon Sep 17 00:00:00 2001
>>>>> From: Sylvestre Ledru<sylves...@debian.org>
>>>>> Date: Sun, 14 May 2017 11:37:37 +0200
>>>>> Subject: [PATCH 5/5] 2017-05-14 Sylvestre Ledru<sylves...@debian.org>
>>>>>
>>>>> * lto-wrapper.c (copy_file): Fix resource leaks
>>>>> CID 1407987, 1407986
>>>> Doesn't this still leak in the cases were we call fatal_error?
>>>
>>> fatal_error is a noreturn function, why should we bother to do any cleanups
>>> after it? All that code is going to be optimized away anyway.
>> But cleaning this kind of thing up does help static analyzers and such.
>> ISTM that we'd need a compelling reason _not_ to accept this kind of patch.
>
> Are the static analyzers so dumb to report something like that?
>
> Unless we have a proof that they are, I think the original short patch is
> the way to go, rather than the much more complicated later patch.
The only thing that's complicated is how the formatting got mucked up in
the patch :( It's literally just adding suitable fclose calls in the
proper paths.
I'm willing to go with the original in the hopes that it's enough to
silence Coverity, but if Coverity still complains we should probably
clean up the formatting and go with the latter patch.
Sylvestre, please commit your original patch with this ChangeLog entry:
* lto-wrapper.c (copy_file) Close both file descriptors before
exiting normally.
Jeff
