[PATCH 1/5] hexdump-parse.c: Fix Resource leak (CID #26032)

[Chris Johns]

On 16/3/21 6:55 am, Joel Sherrill wrote:
> 
> 
> On Mon, Mar 15, 2021 at 2:46 PM Gedare Bloom <gedare at rtems.org
> <mailto:gedare at rtems.org>> wrote:
> 
>     On Sun, Mar 14, 2021 at 8:27 PM Chris Johns <chrisj at rtems.org
>     <mailto:chrisj at rtems.org>> wrote:
>     >
>     > On 13/3/21 2:18 am, Ryan Long wrote:
>     > > CID 26032: Resource leak in rtems_shell_hexdump_rewrite().
>     > >
>     > > Closes #4296
>     > > ---
>     > >  cpukit/libmisc/shell/hexdump-parse.c | 3 +++
>     > >  1 file changed, 3 insertions(+)
>     > >
>     > > diff --git a/cpukit/libmisc/shell/hexdump-parse.c
>     b/cpukit/libmisc/shell/hexdump-parse.c
>     > > index 88b9d56..5b56bbf 100644
>     > > --- a/cpukit/libmisc/shell/hexdump-parse.c
>     > > +++ b/cpukit/libmisc/shell/hexdump-parse.c
>     > > @@ -462,6 +462,9 @@ isint2:                                 
>      switch(fu->bcnt) {
>     > >               (void)printf("\n");
>     > >       }
>     > >  #endif
>     > > +#ifdef __rtems__
>     > > +     free(nextpr);
>     > > +#endif
>     >
>     > I know this has not been done in imported code in rtems.git before but
>     should we
>     > adopt the libbsd standard of adding /* __rtems__ */ to the #else and #endif?
> 
>     Probably, but we also clone-and-own this shell/ code, and we should
>     not bother with these #ifdefs in there. I think I have said this 3
>     times this past week about shell/. The upstream does not want our
>     changes, and we don't import from them anymore.
> 
> Some of these files have massive changes and some don't. Ryan and 
> I looked at main_cp.c and it had at least 40 revisions since the version
> we have. The same Coverity issue appeared to be present but the variable
> names were changed and much clearer.

Yes.

> Chris imported this code initially. I'll trust his ruling on this since I assume
> he is likely to either be the one to update it eventually or have to help someone
> a lot. :)

If the code was updated I would use the libbsd way of handing it. It has been
proven to work.

> And some of it is close enough that the ifdef's are worth it. Some isn't. 
> Hard call on the overall value.

For the existing code it is hard to call. If I was starting again I would say we
had to support a clean separation.

Chris