[PATCH libXpm] After fdopen(), use fclose() instead of close() in error path

[Alan Coopersmith]

On 10/11/18 12:13 PM, Walter Harms wrote:
> 
> 
>> Alan Coopersmith <alan.coopersmith at oracle.com> hat am 1. Oktober 2018 um 00:14
>> geschrieben:
>>
>>
>> Found by Oracle's Parfait 2.2 static analyzer:
>>
>> Error: File Leak
>>     File Leak [file-ptr-leak]:
>>        Leaked File fp
>>          at line 94 of lib/libXpm/src/RdFToBuf.c in function
>> 'XpmReadFileToBuffer
>> '.
>>            fp initialized at line 86 with fdopen
>>            fp leaks when len < 0 at line 92.
>>
>> Introduced-by: commit 8b3024e6871ce50b34bf2dff924774bd654703bc
>>
>> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
>> ---
>>   src/RdFToBuf.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/RdFToBuf.c b/src/RdFToBuf.c
>> index 69e3347..1b386f8 100644
>> --- a/src/RdFToBuf.c
>> +++ b/src/RdFToBuf.c
>> @@ -86,15 +86,15 @@ XpmReadFileToBuffer(
>>       fp = fdopen(fd, "r");
>>       if (!fp) {
>>   	close(fd);
>>   	return XpmOpenFailed;
>>       }
>>       len = stats.st_size;
>>       if (len < 0 || len >= SIZE_MAX) {
>> -	close(fd);
>> +	fclose(fp);
>>   	return XpmOpenFailed;
>>       }
> 
> IMHO it should do both,
> otherwise you have a different behavier when
> fdopen failed and returning XpmOpenFailed
> or size check failed and returning XpmOpenFailed

Once fp = fdopen(fd, ...) succeeds, then fclose(fp) will also close the
underlying fd, so us trying to close it again will cause EBADF errors in
most cases, or possible race conditions if libXpm is being used in a
multi-threaded program that opens another fd in another thread at the
same time.

-- 
	-Alan Coopersmith-               alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - https://blogs.oracle.com/alanc