change log for rtems (2011-03-15)

[Ralf Corsepius]

On 03/15/2011 04:10 PM, rtems-vc at rtems.org wrote:

> 2011-03-15	Joel Sherrill<joel.sherrill at oarcorp.com>
>
> 	* libmisc/shell/main_setenv.c: Address Coverity issue 134 which
> 	recommends using strnXXX methods.
>
> M 1.2778  cpukit/ChangeLog
> M    1.4  cpukit/libmisc/shell/main_setenv.c
>
> diff -u rtems/cpukit/ChangeLog:1.2777 rtems/cpukit/ChangeLog:1.2778
> --- rtems/cpukit/ChangeLog:1.2777	Tue Mar 15 09:52:31 2011
> +++ rtems/cpukit/ChangeLog	Tue Mar 15 10:02:41 2011
> @@ -1,3 +1,8 @@
> +2011-03-15	Joel Sherrill<joel.sherrill at oarcorp.com>
> +
> +	* libmisc/shell/main_setenv.c: Address Coverity issue 134 which
> +	recommends using strnXXX methods.
> +
>   2011-03-15	Sebastian Huber<sebastian.huber at embedded-brains.de>
>
>   	* libmisc/cpuuse/cpuusagereport.c: Avoid assumptions on execution
>
> diff -u rtems/cpukit/libmisc/shell/main_setenv.c:1.3 rtems/cpukit/libmisc/shell/main_setenv.c:1.4
> --- rtems/cpukit/libmisc/shell/main_setenv.c:1.3	Sat Aug 28 15:10:00 2010
> +++ rtems/cpukit/libmisc/shell/main_setenv.c	Tue Mar 15 10:02:41 2011
> @@ -16,6 +16,13 @@
>   #include<rtems/shell.h>
>   #include "internal.h"
>
> +/*
> + *  Limit examining or copying more than 256 characters at a time.
> + *  Yes, this is very arbitrary.  If there are POSIX constants, then
> + *  they should be used.
> + */
> +#define MAX 156
> +
>   int rtems_shell_main_setenv(int argc, char *argv[])
>   {
>     char* env = NULL;
> @@ -32,7 +39,7 @@
>     env = argv[1];
>
>     for (arg = 2; arg<  argc; arg++)
> -    len += strlen(argv[arg]);
> +    len += strnlen(argv[arg], MAX);
>
>     len += argc - 2 - 1;
>
> @@ -44,8 +51,8 @@
>     }
>
>     for (arg = 2, p = string; arg<  argc; arg++) {
> -    strcpy(p, argv[arg]);
> -    p += strlen(argv[arg]);
> +    strncpy(p, argv[arg], MAX);
> +    p += strnlen(argv[arg], MAX);
>       if (arg<  (argc - 1)) {
>         *p = ' ';
>         p++;
>

argv is defined as being 0 terminated
=> using strnlen is wrong.
=> Coverity is wrong

Also, by limiting the string you are
* causing bugs
* introducing memory bloat (a static buffer).

IMO this patch is simply plain wrong and should be reverted.