[scan-admin at coverity.com: New Defects reported by Coverity Scan for Das U-Boot]

[Heinrich Schuchardt]

Am 22. Januar 2021 09:54:20 MEZ schrieb Sughosh Ganu <sughosh.ganu at linaro.org>:
>On Thu, 21 Jan 2021 at 19:14, Heinrich Schuchardt <xypron.glpk at gmx.de>
>wrote:
>
>> On 21.01.21 12:36, Sughosh Ganu wrote:
>> >
>> >
>> > On Thu, 21 Jan 2021 at 00:34, Tom Rini <trini at konsulko.com
>> > <mailto:trini at konsulko.com>> wrote:
>> >
>> >     I decided to run Coverity part-way through the merge window
>this time
>> >     and here's what's been found so far.
>> >
>> >     ----- Forwarded message from scan-admin at coverity.com
>> >     <mailto:scan-admin at coverity.com> -----
>> >
>> >     Date: Mon, 18 Jan 2021 17:53:19 +0000 (UTC)
>> >     From: scan-admin at coverity.com <mailto:scan-admin at coverity.com>
>> >     To: tom.rini at gmail.com <mailto:tom.rini at gmail.com>
>> >     Subject: New Defects reported by Coverity Scan for Das U-Boot
>> >
>> >     Hi,
>> >
>> >     Please find the latest report on new defect(s) introduced to
>Das
>> >     U-Boot found with Coverity Scan.
>> >
>> >     23 new defect(s) introduced to Das U-Boot found with Coverity
>Scan.
>> >     2 defect(s), reported by Coverity Scan earlier, were marked
>fixed in
>> >     the recent build analyzed by Coverity Scan.
>> >
>> >     New defect(s) Reported-by: Coverity Scan
>> >     Showing 20 of 23 defect(s)
>> >
>> >     ** CID 316356:  Resource leaks  (RESOURCE_LEAK)
>> >     /tools/mkeficapsule.c: 225 in add_public_key()
>> >
>> >
>> > <snip>
>> >
>> >
>> >
>> >
>> 
>________________________________________________________________________________________________________
>> >     *** CID 316356:  Resource leaks  (RESOURCE_LEAK)
>> >     /tools/mkeficapsule.c: 225 in add_public_key()
>> >     219             if (ret < 0) {
>> >     220                     fprintf(stderr, "%s: Unable to add
>public
>> >     key to the FDT\n",
>> >     221                             __func__);
>> >     222                     goto err;
>> >     223             }
>> >     224
>> >     >>>     CID 316356:  Resource leaks  (RESOURCE_LEAK)
>> >     >>>     Handle variable "srcfd" going out of scope leaks the
>handle.
>> >     225             return 0;
>> >     226
>> >     227     err:
>> >     228             if (sptr)
>> >     229                     munmap(sptr, src_size);
>> >     230
>> >
>> >
>> > I think these should not cause any issues, since the function
>return
>> > results in the process termination in both the scenarios of success
>and
>> > failure. But i will post a patch to handle these errors to keep the
>> > resource handling consistent.
>>
>> Looking at line 234f:
>>
>>         if (srcfd >= 0)
>>                 close(srcfd);
>>
>> The comparison is wrong. It should be:
>>
>>         if (srcfd != -1)
>>                 close(srcfd);
>>
>> The open.2 man-page says that only -1 signals an error. According to
>the
>> man-page -2 is a legal value for a file descriptor.
>>
>
>Can you point me to which man page you are referring to. The open
>manpage
>on my ubuntu system has the following,
>
>"The return value of open() is a file descriptor, a small, nonnegative
>integer that is used in subsequent system calls"
>
>I could not find any mention of -2 being a valid file descriptor.
>
>-sughosh


You are right

https://pubs.opengroup.org/onlinepubs/9699919799/

says the return value must be positive or -1 (in case of an error).