Re: [Xen-devel] [PATCH v3 5/5] mkelf32: Close those file descriptors in the error paths.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.02.16 at 22:12, <konrad.wilk@xxxxxxxxxx> wrote:
> On Thu, Feb 18, 2016 at 09:45:30AM -0700, Jan Beulich wrote:
>> >>> On 18.02.16 at 17:39, <Ian.Jackson@xxxxxxxxxxxxx> wrote:
>> > Jan Beulich writes ("Re: [PATCH v3 5/5] mkelf32: Close those file 
> descriptors 
>> > in the error paths."):
>> >> On 12.02.16 at 04:08, <konrad.wilk@xxxxxxxxxx> wrote:
>> >> > While we are operating here we may as well fix some of the
>> >> > file descriptor leaks.
>> >> 
>> >> I'm not convinced. The added goto-s make the code uglier to read,
>> >> and this being a standalone utility there's not really any leak here.
>> > 
>> > I don't buy this `uglier to read'.  What `return 1' does is make me
>> > think `is some resource being leaked' and `do I need to audit this
>> > thing'.
>> 
>> Certainly a matter of taste to some degree - goto-s are always
>> ugly to read to my eyes. Irrespective of this I don't buy the leak
>> aspect for a non-library like, short running build utility. The close()
>> calls are just more code, with absolutely no added benefit to the
>> system the code runs on.
> 
> <chuckles>
> 
> If I turned them in:
> 
>       if (..blah..)
>       {
>               close(infd);
>               return -1;
>       }
> 
> would that satisfy you?

Since presumably this would be on a number of error paths, I'm
afraid ...

> (Irrespective of the 'no added benefit to the system the code runs
> on.').

... this aspect would still be an relevant one.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel