[PATCH 2/3] cpukit/libmisc/rtems-fdt: Fixes leaked variable 'bf' in rtems-fdt.c

[Joel Sherrill]

On Tue, Jun 29, 2021 at 3:15 PM Harrison Gerber <hgerber at uccs.edu> wrote:
>
> Joel,
>
> I'm not too sure what is meant by almost duplicating the base file name. The file referenced is 'rtems-fdt.c', which is found in a file called 'rtems-fdt' inside libmisc, so I thought it was best to have the entire directory path at the front and the name of the file being altered in the description. Let me know if this is not correct; I'm not totally sure what is meant here, but the rest I understand! This also is important to your response to the third patch in this series.
>

basename is a POSIX command/term for part of the path. dirname is the
directory portion.

/home/joel/source.c

dirname portion -> /home/joel
basename portion -> source.c

cpukit/libmisc/rtems-fdt: Fixes leaked variable 'bf' in rtems-fdt.c

I see now that the directory rtems-fdt is similar to the source file
so it wasn't quite as redundant as I thought.

How about this?

cpukit/libmisc/rtems-fdt/rtems-fdt.c: close() file to avoid leaking descriptor

That makes the filename clear and doesn't waste characters with "in "

Personally, if the comment needed to longer, I would be ok ditching
the cpukit/ part but you don't need to do that for space.

> Thanks for the quick response!
>

No problem. It takes a village to review patches. :)

--joel

> Harrison Gerber
> hgerber at uccs.edu
> 720-288-7308
>
> -----Original Message-----
> From: Joel Sherrill <joel at rtems.org>
> Sent: Tuesday, June 29, 2021 2:09 PM
> To: Harrison Edward Gerber <gerberhe11 at gmail.com>
> Cc: rtems-devel at rtems.org <devel at rtems.org>; Harrison Gerber <hgerber at uccs.edu>
> Subject: Re: [PATCH 2/3] cpukit/libmisc/rtems-fdt: Fixes leaked variable 'bf' in rtems-fdt.c
>
> I'm ok with the fix but the commit message could be improved.
> Something like this would be better.
>
> cpukit/libmisc/rtems-fdt.c : close() file to avoid leaking descriptor
>
> You almost duplicated the base file name and it isn't leaking the variable, it is leaking the file descriptor referenced by bf.
>
> --joel
>
> On Tue, Jun 29, 2021 at 2:54 PM Harrison Edward Gerber <gerberhe11 at gmail.com> wrote:
> >
> > See Also CID 1437645
> >
> > Closes #4297
> > ---
> >  cpukit/libmisc/rtems-fdt/rtems-fdt.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> > b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> > index bfbc6102a2..5580d415e2 100644
> > --- a/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> > +++ b/cpukit/libmisc/rtems-fdt/rtems-fdt.c
> > @@ -611,6 +611,7 @@ rtems_fdt_load (const char* filename, rtems_fdt_handle* handle)
> >      return fe;
> >    }
> >
> > +  close (bf);
> >    return 0;
> >  }
> >
> > --
> > 2.25.1
> >
> > _______________________________________________
> > devel mailing list
> > devel at rtems.org
> > http://lists.rtems.org/mailman/listinfo/devel