[PATCH] Psxtest : Fix String Turncation warning

[Joel Sherrill]

On Wed, Aug 12, 2020 at 9:43 AM Aschref Ben-Thabet <
aschref.ben-thabet at embedded-brains.de> wrote:

> Hello joel, Gedare,
>
>   i think in this way , we may use Strdup/ Strrndup instead of memcpy to
> guarantee the copy of null terminated String.
> in this case we don't need to allocate memory, since Strdup do this one.
>

memcpy() copies bytes from one location to another and  str*dup() creates
a second copy with a newly allocated location. I still don't see the
functional
equivalence to do a replacement.

And we still haven't seen the compiler report from the original code that we
are trying to address.

>
>   i m unconscious with the line #224 key.dsize = sizeof( test_strings )
> then i have a doubt if it is correctly defined or we take in this case
> the size of pointer instead of the size of an array.
>

That is a common programming mistake. If that's what is happening,
then the sizeof() probably should be strlen() or the string needs to be
declared with a fixed size and the same constant used on the right hand
size.

Sorry. I haven't seen the actual warning and the fix doesn't seem like
the right way to manage strings.

--joel

>
> Best regards,
> Aschref
>
> On 8/12/20 3:55 PM, Joel Sherrill wrote:
> >
> >
> > On Wed, Aug 12, 2020 at 8:41 AM Gedare Bloom <gedare at rtems.org
> > <mailto:gedare at rtems.org>> wrote:
> >
> >     On Wed, Aug 12, 2020 at 7:03 AM Joel Sherrill <joel at rtems.org
> >     <mailto:joel at rtems.org>> wrote:
> >      >
> >      >
> >      >
> >      > On Wed, Aug 12, 2020 at 7:07 AM Aschref Ben-Thabet
> >     <aschref.ben-thabet at embedded-brains.de
> >     <mailto:aschref.ben-thabet at embedded-brains.de>> wrote:
> >      >>
> >      >> From: Aschref Ben Thabet <aschref.ben-thabet at embedded-brains.de
> >     <mailto:aschref.ben-thabet at embedded-brains.de>>
> >      >>
> >      >> replace strncpy with memcpy to silence this warning and free the
> >      >> allocated memory block.
> >      >
> >      >
> >      > I don't see a call to strncpy being replaced. Maybe I need
> >     coffee. I see an
> >      > RTEMS test assert strcmp.
> >      >
> >      > Silence what warning?
> >      >
> >      > I do not  think it is appropriate to replace str*cpy with memcpy.
> >     What is the warning?
> >      >
> >
> >     We had this discussion on a previous thread, start from
> >     https://lists.rtems.org/pipermail/devel/2020-July/061008.html
> >
> >
> > I know and I didn't like it then but couldn't put my finger on it.  Plus
> > some of the warnings have been in psxhdr tests and my concern is
> > lower because they never get executed.
> >
> > Using memcpy() violates the contract on how strings are to be processed.
> > There is no assurance we end up with NULL terminated strings. We have
> > seen string warnings with Coverity and other scanners before and
> addressed
> > them without abandoning str*(). Why now?
> >
> > What is the warning and what is the proper fix? Eliminating use of string
> > operations is heavy handed and introduces other risks.
> >
> > We haven't seen the actual warning and alternative solutions.
> >
> > FWIW I've had discussions with aviation safety reviewers and, to my
> > surprise, strcpy() is actually OK to use sometimes. I asked because
> > strncpy() is not in the FACE Technical Standard profiles because I don't
> > think
> > it was not in POSIX 2001 which is what the baseline API sets were based
> on.
> >
> > --joel
> >
> >
> >
> >
> >      >> ---
> >      >>  testsuites/psxtests/psxndbm01/init.c | 3 ++-
> >      >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >      >>
> >      >> diff --git a/testsuites/psxtests/psxndbm01/init.c
> >     b/testsuites/psxtests/psxndbm01/init.c
> >      >> index b524aff0df..658af58df3 100644
> >      >> --- a/testsuites/psxtests/psxndbm01/init.c
> >      >> +++ b/testsuites/psxtests/psxndbm01/init.c
> >      >> @@ -216,11 +216,12 @@ rtems_task Init(rtems_task_argument
> ignored)
> >      >>    get_phone_no = dbm_fetch( db, name2 );
> >      >>    rtems_test_assert( strcmp( (const char*)get_phone_no.dptr,
> >     PHONE_NO2 ) == 0 );
> >      >>
> >      >> -  puts( "Fetch non-existing record and confirm error." );
> >      >> +  puts( "Fetch non-existing record and confirm error." );
> >      >
> >      >
> >      > I don't see a change here.
> >      >
> >      > And while you are here non-existing isn't a word. It should be
> >     "nonexistent"
> >      >
> >      >>
> >      >>    test_strings = (char*)malloc(6);
> >      >>    memcpy( test_strings, "Hello", 5 );
> >      >>
> >      >>    test_strings[5] = '\0';
> >      >> +  free(test_strings);
> >      >>
> >      >>  /* The data pointed by test_string is now pointed by key.dptr */
> >      >>    key.dptr = test_strings;
> >      >> --
> >      >> 2.26.2
> >      >>
> >      >> _______________________________________________
> >      >> devel mailing list
> >      >> devel at rtems.org <mailto:devel at rtems.org>
> >      >> http://lists.rtems.org/mailman/listinfo/devel
> >      >
> >      > _______________________________________________
> >      > devel mailing list
> >      > devel at rtems.org <mailto: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/20200812/b59348ba/attachment.html>