On 16/10/2024 18.22, Daniel P. Berrangé wrote:
On Wed, Oct 16, 2024 at 06:07:12PM +0200, Thomas Huth wrote:
The linker on OpenBSD complains:

  ld: warning: dirtyrate.c:447 (../src/migration/dirtyrate.c:447)(...):
  warning: strcpy() is almost always misused, please use strlcpy()

Is that the only place it complains ?  We use 'strcpy' in almost
100 places across the codebase....

There are only a fistful of other warnings. I guess most of the spots are turned into inlined code by the compiler, so the linker never sees those other occurrences.

It's currently not a real problem in this case since both arrays
have the same size (256 bytes). But just in case somebody changes
the size of the source array in the future, let's better play safe
and use g_strlcpy() here instead.

Signed-off-by: Thomas Huth <th...@redhat.com>
---
  migration/dirtyrate.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 233acb0855..090c76e934 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -444,7 +444,7 @@ static void get_ramblock_dirty_info(RAMBlock *block,
      info->ramblock_pages = qemu_ram_get_used_length(block) >>
                             qemu_target_page_bits();
      info->ramblock_addr = qemu_ram_get_host_addr(block);
-    strcpy(info->idstr, qemu_ram_get_idstr(block));
+    g_strlcpy(info->idstr, qemu_ram_get_idstr(block), sizeof(info->idstr));
  }

Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>


Is it worth also adding

   G_STATIC_ASSERT(sizeof((struct RamblockDirtyInfo){}.idstr) ==
                   sizeof((struct RAMBlock){}.idstr));

at the top of this file, since both of these fields are expected to
be the same size by this code, to avoid truncation.

... or alternatively check the return value of g_strlcpy() ? ... but that wouldn't work if pstrcpy() if we switch to that function instead.

I don't mind either way - Peter, Fabiano, Hyman, what's your opinion here?

 Thomas


Reply via email to