change log for rtems (2011-03-15)

[Joel Sherrill]

On 03/15/2011 10:27 AM, Ralf Corsepius wrote:
> 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).
>
There are no static buffers and none were introduced.
There was a malloc of the buffer.


> IMO this patch is simply plain wrong and should be reverted.
>
OK... anyone else have an opinion?
> _______________________________________________
> rtems-vc mailing list
> rtems-vc at rtems.org
> http://www.rtems.org/mailman/listinfo/rtems-vc


-- 
Joel Sherrill, Ph.D.             Director of Research&  Development
joel.sherrill at OARcorp.com        On-Line Applications Research
Ask me about RTEMS: a free RTOS  Huntsville AL 35805
    Support Available             (256) 722-9985