Re: [PATCH] migration/dirtyrate: Silence warning about strcpy() on OpenBSD

[Thomas Huth <thuth@redhat.com>]

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 <thuth@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é <berrange@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