[PATCH] Psxtest : Fix String Turncation warning

[Joel Sherrill]

On Wed, Aug 12, 2020 at 8:41 AM Gedare Bloom <gedare at rtems.org> wrote:

> On Wed, Aug 12, 2020 at 7:03 AM Joel Sherrill <joel at rtems.org> wrote:
> >
> >
> >
> > On Wed, Aug 12, 2020 at 7:07 AM Aschref Ben-Thabet <
> aschref.ben-thabet at embedded-brains.de> wrote:
> >>
> >> From: Aschref Ben Thabet <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
> >> http://lists.rtems.org/mailman/listinfo/devel
> >
> > _______________________________________________
> > devel mailing list
> > 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/1439faa9/attachment.html>