[PATCH v2 1/4] main_cp.c: Unused value (CID #1255344)

Ryan Long ryan.long at oarcorp.com
Tue Apr 6 19:23:54 UTC 2021


The Coverity issue says:

assigned_value: Assigning value from rval to badcp here, but that stored value is overwritten before it can be used.

I thought this would apply to the sections with a 'break' in them, but those sections aren't being marked by Coverity. I'll take those out and resend it.

-----Original Message-----
From: Gedare Bloom <gedare at rtems.org> 
Sent: Tuesday, April 6, 2021 2:17 PM
To: Ryan Long <ryan.long at oarcorp.com>
Cc: devel at rtems.org
Subject: Re: [PATCH v2 1/4] main_cp.c: Unused value (CID #1255344)

On Tue, Apr 6, 2021 at 12:49 PM Ryan Long <ryan.long at oarcorp.com> wrote:
>
> CID 1255344: Unused value in copy().
>
> Closes #4339
> ---
>  cpukit/libmisc/shell/main_cp.c | 88 
> ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
>
> diff --git a/cpukit/libmisc/shell/main_cp.c 
> b/cpukit/libmisc/shell/main_cp.c index cddbc95..3f2b100 100644
> --- a/cpukit/libmisc/shell/main_cp.c
> +++ b/cpukit/libmisc/shell/main_cp.c
> @@ -310,11 +310,31 @@ copy(rtems_shell_cp_globals* cp_globals,
>                 case FTS_ERR:
>                         warnx("%s: %s",
>                             curr->fts_path, 
> strerror(curr->fts_errno));
> +               #if __rtems__
> +               /*
> +                * Coverity spotted that badcp is set by each loop
> +                * iteration so setting it right before continue
> +                * results in the value being unused. See CID 1255344
> +                *
> +                * The current NetBSD source (v1.62) was checked and
> +                * the same issue appears to apply although the
> +                * variable names have changed since this was imported
> +                * to RTEMS.
> +                *
> +                * This pattern exists in multiple places in this file.
> +                */
> +                       rval = 1;
> +               #else
>                         badcp = rval = 1;
> +               #endif
>                         continue;
>                 case FTS_DC:                    /* Warn, continue. */
>                         warnx("%s: directory causes a cycle", 
> curr->fts_path);
> +               #if __rtems__
> +                       rval = 1;
> +               #else
>                         badcp = rval = 1;
> +               #endif
>                         continue;
>                 default:
>                         ;
> @@ -366,7 +386,11 @@ copy(rtems_shell_cp_globals* cp_globals,
>                         if (target_mid - to.p_path + nlen >= PATH_MAX) {
>                                 warnx("%s%s: name too long (not copied)",
>                                     to.p_path, p);
> +                       #if __rtems__
> +                               rval = 1;
> +                       #else
>                                 badcp = rval = 1;
> +                       #endif
>                                 continue;
>                         }
>                         (void)strncat(target_mid, p, nlen); @@ -418,7 
> +442,11 @@ copy(rtems_shell_cp_globals* cp_globals,
>                             to_stat.st_ino == curr->fts_statp->st_ino) {
>                                 warnx("%s and %s are identical (not copied).",
>                                     to.p_path, curr->fts_path);
> +                       #if __rtems__
> +                               rval = 1;
> +                       #else
>                                 badcp = rval = 1;
> +                       #endif
>                                 if (S_ISDIR(curr->fts_statp->st_mode))
>                                         (void)fts_set(ftsp, curr, FTS_SKIP);
>                                 continue; @@ -428,7 +456,11 @@ 
> copy(rtems_shell_cp_globals* cp_globals,
>                                 warnx("cannot overwrite directory %s with "
>                                     "non-directory %s",
>                                     to.p_path, curr->fts_path);
> +                       #if __rtems__
> +                               rval = 1;
> +                       #else
>                                 badcp = rval = 1;
> +                       #endif
>                                 continue;
>                         }
>                         dne = 0;
> @@ -441,10 +473,24 @@ copy(rtems_shell_cp_globals* cp_globals,
>                             ((fts_options & FTS_COMFOLLOW) &&
>                             curr->fts_level == 0)) {
>                                 if (copy_file(cp_globals, curr, dne))
> +                               #if __rtems__
> +                               {
> +                                       badcp = 1;
> +                                       rval = 1;
> +                               }
> +                               #else
>                                         badcp = rval = 1;
> +                               #endif

why change this? Just leave how it is, it is functionally equivalent.
Same goes for the remaining ones below.

>                         } else {
>                                 if (copy_link(cp_globals, curr, !dne))
> +                               #if __rtems__
> +                               {
> +                                       badcp = 1;
> +                                       rval = 1;
> +                               }
> +                               #else
>                                         badcp = rval = 1;
> +                               #endif
>                         }
>                         break;
>                 case S_IFDIR:
> @@ -452,7 +498,14 @@ copy(rtems_shell_cp_globals* cp_globals,
>                                 warnx("%s is a directory (not copied).",
>                                     curr->fts_path);
>                                 (void)fts_set(ftsp, curr, FTS_SKIP);
> +                       #if __rtems__
> +                       {
> +                               badcp = 1;
> +                               rval = 1;
> +                       }
> +                       #else
>                                 badcp = rval = 1;
> +                       #endif
>                                 break;
>                         }
>                         /*
> @@ -482,10 +535,24 @@ copy(rtems_shell_cp_globals* cp_globals,
>                 case S_IFCHR:
>                         if (Rflag) {
>                                 if (copy_special(cp_globals, 
> curr->fts_statp, !dne))
> +                               #if __rtems__
> +                               {
> +                                       badcp = 1;
> +                                       rval = 1;
> +                               }
> +                               #else
>                                         badcp = rval = 1;
> +                               #endif
>                         } else {
>                                 if (copy_file(cp_globals, curr, dne))
> +                               #if __rtems__
> +                               {
> +                                       badcp = 1;
> +                                       rval = 1;
> +                               }
> +                               #else
>                                         badcp = rval = 1;
> +                               #endif
>                         }
>                         break;
>                 case S_IFSOCK:
> @@ -494,15 +561,36 @@ copy(rtems_shell_cp_globals* cp_globals,
>                 case S_IFIFO:
>                         if (Rflag) {
>                                 if (copy_fifo(cp_globals, 
> curr->fts_statp, !dne))
> +                               #if __rtems__
> +                               {
> +                                       badcp = 1;
> +                                       rval = 1;
> +                               }
> +                               #else
>                                         badcp = rval = 1;
> +                               #endif
>                         } else {
>                                 if (copy_file(cp_globals, curr, dne))
> +                               #if __rtems__
> +                               {
> +                                       badcp = 1;
> +                                       rval = 1;
> +                               }
> +                               #else
>                                         badcp = rval = 1;
> +                               #endif
>                         }
>                         break;
>                 default:
>                         if (copy_file(cp_globals, curr, dne))
> +                       #if __rtems__
> +                       {
> +                               badcp = 1;
> +                               rval = 1;
> +                       }
> +                       #else
>                                 badcp = rval = 1;
> +                       #endif
>                         break;
>                 }
>                 if (vflag && !badcp)
> --
> 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