[PATCH 2/3] main_cp.c: Ignore return value from stat()

Joel Sherrill joel at rtems.org
Wed Mar 31 16:36:44 UTC 2021


On Wed, Mar 31, 2021 at 11:24 AM Gedare Bloom <gedare at rtems.org> wrote:

> On Wed, Mar 31, 2021 at 10:21 AM Ryan Long <ryan.long at oarcorp.com> wrote:
> >
> > I'm assuming you accidentally left the (void) in after that #endif, but
> that's the approach that Joel and I did first.  After Joel saw it
> implemented, he said it made the code look too messy to him because of the
> if statements.
> >
> Right. The thing is that we aren't looking to make this code clean,
> we're optimizing for easier updates from the upstream. Generally, it
> is easier to undo and redo small changes than large changes.
>

If you are OK with an ugly #if (void) #endif, then I'm OK with it.

It is  ugly either way.

Make the void ifdef'ed. Sorry about the pendulum swinging.


> > -----Original Message-----
> > From: Gedare Bloom <gedare at rtems.org>
> > Sent: Wednesday, March 31, 2021 11:17 AM
> > To: Ryan Long <ryan.long at oarcorp.com>
> > Cc: devel at rtems.org
> > Subject: Re: [PATCH 2/3] main_cp.c: Ignore return value from stat()
> >
> > On Wed, Mar 31, 2021 at 9:16 AM Ryan Long <ryan.long at oarcorp.com> wrote:
> > >
> > > CID 26051: Unchecked return value from library in main_cp().
> > >
> > > Closes #4365
> > > ---
> > >  cpukit/libmisc/shell/main_cp.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/cpukit/libmisc/shell/main_cp.c
> > > b/cpukit/libmisc/shell/main_cp.c index cddbc95..bb31a35 100644
> > > --- a/cpukit/libmisc/shell/main_cp.c
> > > +++ b/cpukit/libmisc/shell/main_cp.c
> > > @@ -254,10 +254,17 @@ main_cp(rtems_shell_cp_globals* cp_globals, int
> argc, char *argv[])
> > >                  * the initial mkdir().
> > >                  */
> > >                 if (r == -1) {
> > > +                       #ifdef __rtems__
> > > +                       if (Rflag && (Lflag || Hflag))
> > > +                               (void) stat(*argv, &tmp_stat);
> > > +                       else
> > > +                               (void) lstat(*argv, &tmp_stat);
> > > +                       #else
> > >                         if (Rflag && (Lflag || Hflag))
> > >                                 stat(*argv, &tmp_stat);
> > >                         else
> > >                                 lstat(*argv, &tmp_stat);
> > > +                       #endif
> > >
> >
> > Would it be better to keep each change localized? at first I didn't
> really care for this approach that was taken in patch 1, but compared to
> copy-pasting multiple lines of code, it might be simpler to deal with
> fine-grained modifications, like this:
> >
> >                          if (Rflag && (Lflag || Hflag))
> > +                               #ifdef __rtems__
> > +                               (void)
> > +                               #endif
> >                                  (void) stat(*argv, &tmp_stat);
> >
> > >                         if (S_ISDIR(tmp_stat.st_mode) && Rflag)
> > >                                 type = DIR_TO_DNE;
> > > --
> > > 1.8.3.1
> > >
> > > _______________________________________________
> > > devel mailing list
> > > devel at rtems.org
> > > http://lists.rtems.org/mailman/listinfo/devel
> _______________________________________________
> devel mailing list
> devel at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210331/90790239/attachment-0001.html>


More information about the devel mailing list