On Tue, Jun 29, 2021 at 2:26 PM Joel Sherrill <j...@rtems.org> wrote: > > On Tue, Jun 29, 2021 at 3:15 PM Harrison Gerber <hger...@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 > We usually just use two hops, e.g., cpukit/libmisc: close() file to avoid leaking descriptor
whether or not to specify this is in rtems-fdt is a different choice. We don't have a standard way yet of making these "tags" for commits to orient the review/maintainer responsibility. however, we also will not say cpukit for example for something in score, we just say score. Here, I'd be happy with "libmisc/rtems-fdt: 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. +1 ditch it anyway, conciseness in the short-log is a good thing > > > Thanks for the quick response! > > > > No problem. It takes a village to review patches. :) > > --joel > > > Harrison Gerber > > hger...@uccs.edu > > 720-288-7308 > > > > -----Original Message----- > > From: Joel Sherrill <j...@rtems.org> > > Sent: Tuesday, June 29, 2021 2:09 PM > > To: Harrison Edward Gerber <gerberh...@gmail.com> > > Cc: rtems-de...@rtems.org <devel@rtems.org>; Harrison Gerber > > <hger...@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 > > <gerberh...@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@rtems.org > > > http://lists.rtems.org/mailman/listinfo/devel > _______________________________________________ > devel mailing list > devel@rtems.org > http://lists.rtems.org/mailman/listinfo/devel _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
