Re: [PATCH v6 18/19] migration/multifd: Stop changing the packet on recv side

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Aug 27, 2024 at 03:45:11PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > On Tue, Aug 27, 2024 at 02:46:05PM -0300, Fabiano Rosas wrote:
>> >> @@ -254,12 +250,10 @@ int multifd_ram_unfill_packet(MultiFDRecvParams *p, 
>> >> Error **errp)
>> >>          return 0;
>> >>      }
>> >>  
>> >> -    /* make sure that ramblock is 0 terminated */
>> >> -    packet->ramblock[255] = 0;
>> >> -    p->block = qemu_ram_block_by_name(packet->ramblock);
>> >> +    ramblock_name = g_strndup(packet->ramblock, 255);
>> >
>> > I understand we want to move to a const*, however this introduces a 256B
>> > allocation per multifd packet, which we definitely want to avoid.. I wonder
>> > whether that's worthwhile just to make it const. :-(
>> >
>> > I don't worry too much on the const* and vars pointed being abused /
>> > updated when without it - the packet struct is pretty much limited only to
>> > be referenced in this unfill function, and then we will do the load based
>> > on MultiFDRecvParams* later anyway.  So personally I'd rather lose the
>> > const* v.s. one allocation.
>> >
>> > Or we could also sanity check byte 255 to be '\0' (which, AFAIU, should
>> > always be the case..), then we can get both benefits.
>> 
>> We can't because it breaks compat. Previous QEMUs didn't zero the
>> packet.
>
> Ouch!
>
> Then.. shall we still try to avoid the allocation?

Can I strcpy it to the stack?

char idstr[256];

strncpy(&idstr, packet->ramblock, 256);
idstr[255] = 0;