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

Gedare Bloom gedare at rtems.org
Wed Mar 31 16:24:27 UTC 2021


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.

> -----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


More information about the devel mailing list