[PATCH 5/5] rtl-shell.c: Resource leak (CID #1444140)
CID 1444140: Resource leak in rtems_rtl_shell_object(). Closes #4300 --- cpukit/libdl/rtl-shell.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cpukit/libdl/rtl-shell.c b/cpukit/libdl/rtl-shell.c index 9f8a136..bcecdd4 100644 --- a/cpukit/libdl/rtl-shell.c +++ b/cpukit/libdl/rtl-shell.c @@ -733,14 +733,17 @@ rtems_rtl_shell_object (const rtems_printer* printer, int argc, char* argv[]) if (dlinfo (RTLD_SELF, RTLD_DI_UNRESOLVED, &unresolved) < 0) { rtems_printf (printer, "error: %s: %s\n", argv[arg], dlerror ()); + (void) dlclose (handle); return 1; } if (unresolved != 0) { rtems_printf (printer, "warning: unresolved symbols present\n"); + (void) dlclose (handle); return 1; } + (void) dlclose (handle); } else if (strcmp (argv[arg], "unload") == 0) { -- 1.8.3.1 ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 5/5] rtl-shell.c: Resource leak (CID #1444140)
On 13/3/21 2:18 am, Ryan Long wrote: > CID 1444140: Resource leak in rtems_rtl_shell_object(). > > Closes #4300 > --- > cpukit/libdl/rtl-shell.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/cpukit/libdl/rtl-shell.c b/cpukit/libdl/rtl-shell.c > index 9f8a136..bcecdd4 100644 > --- a/cpukit/libdl/rtl-shell.c > +++ b/cpukit/libdl/rtl-shell.c > @@ -733,14 +733,17 @@ rtems_rtl_shell_object (const rtems_printer* printer, > int argc, char* argv[]) > if (dlinfo (RTLD_SELF, RTLD_DI_UNRESOLVED, &unresolved) < 0) > { >rtems_printf (printer, "error: %s: %s\n", argv[arg], dlerror ()); > + (void) dlclose (handle); >return 1; > } > > if (unresolved != 0) > { >rtems_printf (printer, "warning: unresolved symbols present\n"); > + (void) dlclose (handle); >return 1; > } > + (void) dlclose (handle); >} >else if (strcmp (argv[arg], "unload") == 0) >{ > The handle should be not closed in any of these cases. Have you run the command after making these changes and played with command? This shell command is a tool to load and play with loaded modules and one command is to load a module and others let you play with it. The handle address is printed on the console. Coverity may like to track and nag you about things like this but it is not always right. In relation to all these Coverity changes ... are all these changes being tested? Is the boarder context of the change being examined in relation to the specific change? Chris ___ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
Re: [PATCH 5/5] rtl-shell.c: Resource leak (CID #1444140)
On Sun, Mar 14, 2021 at 8:27 PM Chris Johns wrote: > > On 13/3/21 2:18 am, Ryan Long wrote: > > CID 1444140: Resource leak in rtems_rtl_shell_object(). > > > > Closes #4300 > > --- > > cpukit/libdl/rtl-shell.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/cpukit/libdl/rtl-shell.c b/cpukit/libdl/rtl-shell.c > > index 9f8a136..bcecdd4 100644 > > --- a/cpukit/libdl/rtl-shell.c > > +++ b/cpukit/libdl/rtl-shell.c > > @@ -733,14 +733,17 @@ rtems_rtl_shell_object (const rtems_printer* printer, > > int argc, char* argv[]) > > if (dlinfo (RTLD_SELF, RTLD_DI_UNRESOLVED, &unresolved) < 0) > > { > >rtems_printf (printer, "error: %s: %s\n", argv[arg], dlerror ()); > > + (void) dlclose (handle); > >return 1; > > } > > > > if (unresolved != 0) > > { > >rtems_printf (printer, "warning: unresolved symbols present\n"); > > + (void) dlclose (handle); > >return 1; > > } > > + (void) dlclose (handle); > >} > >else if (strcmp (argv[arg], "unload") == 0) > >{ > > > > The handle should be not closed in any of these cases. Have you run the > command > after making these changes and played with command? > > This shell command is a tool to load and play with loaded modules and one > command is to load a module and others let you play with it. The handle > address > is printed on the console. Coverity may like to track and nag you about things > like this but it is not always right. > > In relation to all these Coverity changes ... are all these changes being > tested? Is the boarder context of the change being examined in relation to the > specific change? > I think this is an important, and challenging, question, since some things are not fully tested. In this case, we should identify/create tests to exercise the undefined behavior first, and then commit a change after testing. As the easy coverity issues are being fixed, now we need to be more meticulous since we can't just "think through" the modifications being proposed, such as this one (and the NULL out in shell calls) without understanding the scope of possible invocations. For example, maybe we need a test case for telnet shells, before we make a change that impacts their potential useability. > Chris > ___ > 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