Re: [PATCH 3/5] resolv: Fix file handle leak in __resolv_conf_load [BZ #25429]

[Florian Weimer <fweimer at redhat dot com>]

* Adhemerval Zanella:

> On 21/01/2020 15:41, Florian Weimer wrote:
>> res_vinit_1 did not close the stream on errors, only on success.
>> This change moves closing the stream to __resolv_conf_load, for both
>> the success and error cases.
>> 
>> Fixes commit 89f187a40fc0ad4e22838526bfe34d73f758b776 ("resolv: Use
>> getline for configuration file reading in res_vinit_1") and commit
>> 3f853f22c87f0b671c0366eb290919719fa56c0e ("resolv: Lift domain search
>> list limits [BZ #19569] [BZ #21475]"), where memory allocation was
>> introduced into res_vinit_1.
>> ---
>>  resolv/res_init.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>> 
>> diff --git a/resolv/res_init.c b/resolv/res_init.c
>> index 95dce098aa..09345718cd 100644
>> --- a/resolv/res_init.c
>> +++ b/resolv/res_init.c
>> @@ -508,7 +508,6 @@ res_vinit_1 (FILE *fp, struct resolv_conf_parser *parser)
>>                continue;
>>              }
>>          }
>> -      fclose (fp);
>>      }
>>    if (__glibc_unlikely (nameserver_list_size (&parser->nameserver_list) == 0))
>>      {
>> @@ -593,6 +592,13 @@ __resolv_conf_load (struct __res_state *preinit)
>>      }
>>    resolv_conf_parser_free (&parser);
>>  
>> +  if (fp != NULL)
>> +    {
>> +      int saved_errno = errno;
>> +      fclose (fp);
>> +      __set_errno (saved_errno);
>> +    }
>> +
>>    return conf;
>>  }
>
> Why not close the FILE on __resolv_conf_load? It make the FILE object 
> cleanup as close as its creation, which usually improves readability.

Sorry, I don't understand.  Isn't this what the patch does?

Thanks,
Florian