On Wed, Aug 12, 2020 at 9:43 AM Aschref Ben-Thabet < aschref.ben-tha...@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 <ged...@rtems.org > > <mailto:ged...@rtems.org>> wrote: > > > > On Wed, Aug 12, 2020 at 7:03 AM Joel Sherrill <j...@rtems.org > > <mailto:j...@rtems.org>> wrote: > > > > > > > > > > > > On Wed, Aug 12, 2020 at 7:07 AM Aschref Ben-Thabet > > <aschref.ben-tha...@embedded-brains.de > > <mailto:aschref.ben-tha...@embedded-brains.de>> wrote: > > >> > > >> From: Aschref Ben Thabet <aschref.ben-tha...@embedded-brains.de > > <mailto:aschref.ben-tha...@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@rtems.org <mailto:devel@rtems.org> > > >> http://lists.rtems.org/mailman/listinfo/devel > > > > > > _______________________________________________ > > > devel mailing list > > > devel@rtems.org <mailto:devel@rtems.org> > > > http://lists.rtems.org/mailman/listinfo/devel > > >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel
