<div dir="ltr">My recollection is that memcpy is undefined on overlapping memory regions.<div>You should use memmove.</div><div><br></div><div>A bigger question is why do these overlap and the code still thinks the copy is</div><div>needed.</div><div><br></div><div>--joel</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 30, 2020 at 10:37 AM Gedare Bloom <<a href="mailto:gedare@rtems.org">gedare@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">This looks good to me. memcpy followed by explicit delimiter<br>
assignment is better than strncpy. however, note below:<br>
<br>
On Thu, Jul 30, 2020 at 5:36 AM Aschref Ben-Thabet<br>
<<a href="mailto:aschref.ben-thabet@embedded-brains.de" target="_blank">aschref.ben-thabet@embedded-brains.de</a>> wrote:<br>
><br>
> From: Aschref Ben Thabet <<a href="mailto:aschref.ben-thabet@embedded-brains.de" target="_blank">aschref.ben-thabet@embedded-brains.de</a>><br>
><br>
> GCC 10 warns about an overlapping using strncpy.<br>
> -> Replace some calls of strncpy with a memcpy to avoid this issue.<br>
> ---<br>
> cpukit/libblock/src/bdpart-mount.c | 4 ++--<br>
> testsuites/psxtests/psxndbm01/init.c | 2 +-<br>
> 2 files changed, 3 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/cpukit/libblock/src/bdpart-mount.c b/cpukit/libblock/src/bdpart-mount.c<br>
> index cfc08ead30..f689b18ebb 100644<br>
> --- a/cpukit/libblock/src/bdpart-mount.c<br>
> +++ b/cpukit/libblock/src/bdpart-mount.c<br>
> @@ -54,7 +54,7 @@ rtems_status_code rtems_bdpart_mount(<br>
> if (logical_disk_name == NULL) {<br>
> return RTEMS_NO_MEMORY;<br>
> }<br>
> - strncpy( logical_disk_name, disk_name, disk_name_size);<br>
> + memcpy( logical_disk_name, disk_name, disk_name_size);<br>
><br>
is it guaranteed to have a NUL delimiter in disk_name up to<br>
disk_name_size? (I don't think it does.)<br>
<br>
It should be safer to follow this with:<br>
logical_disk_name[disk_name_size] = '\0';<br>
<br>
> /* Get disk file name */<br>
> if (disk_file_name != NULL) {<br>
> @@ -148,7 +148,7 @@ rtems_status_code rtems_bdpart_unmount(<br>
> esc = RTEMS_NO_MEMORY;<br>
> goto cleanup;<br>
> }<br>
> - strncpy( mount_point, mount_base, mount_base_size);<br>
> + memcpy( mount_point, mount_base, mount_base_size);<br>
> mount_point [mount_base_size] = '/';<br>
> strncpy( mount_point + mount_base_size + 1, disk_file_name, disk_file_name_size);<br>
<br>
I guess this one doesn't give a warning? Would it still make any sense<br>
to replace it?<br>
<br>
><br>
> diff --git a/testsuites/psxtests/psxndbm01/init.c b/testsuites/psxtests/psxndbm01/init.c<br>
> index a13afa7315..b524aff0df 100644<br>
> --- a/testsuites/psxtests/psxndbm01/init.c<br>
> +++ b/testsuites/psxtests/psxndbm01/init.c<br>
> @@ -218,7 +218,7 @@ rtems_task Init(rtems_task_argument ignored)<br>
><br>
> puts( "Fetch non-existing record and confirm error." );<br>
> test_strings = (char*)malloc(6);<br>
> - strncpy( test_strings, "Hello", 5 );<br>
> + memcpy( test_strings, "Hello", 5 );<br>
><br>
> test_strings[5] = '\0';<br>
><br>
> --<br>
> 2.26.2<br>
><br>
> _______________________________________________<br>
> devel mailing list<br>
> <a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
_______________________________________________<br>
devel mailing list<br>
<a href="mailto:devel@rtems.org" target="_blank">devel@rtems.org</a><br>
<a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
</blockquote></div>