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

[Sughosh Ganu]

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