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

[Gedare Bloom]

On Mon, Mar 15, 2021 at 5:14 PM Chris Johns <chrisj at rtems.org> wrote:
>
> On 16/3/21 10:07 am, Joel Sherrill wrote:
> > On Mon, Mar 15, 2021 at 6:01 PM Chris Johns <chrisj at rtems.org
> > <mailto:chrisj at rtems.org>> wrote:
> >     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>
> >     > <mailto: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>
> >     >     <mailto: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.
> >
> >
> > I assume you would checkout the original version and diff it. But would the
> > ifdef rtems ease the burden any?
>
> Yes and a good question, I do not know. I suppose it would depend on the file
> and changes.
>
> > I tend to like going with them otherwise, we have some files we do it
> > on and others we don't.
>
> This makes sense so if we feel it would help even now lets add them and if we
> add then I feel it is only small amount of extra effort to add them in the
> libbsd style.
>

It is good to set consistent rules at any rate.

> Chris