On 10/14/25 14:33, Peter Krempa wrote:
> On Tue, Oct 14, 2025 at 08:31:47 +0200, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <[email protected]>
>>
>> The main difference is that wmem_packet_scope() is gone [1] but
>> the packet_info struct has 'pool` member which points to the
>> allocator used for given packet.
>>
>> Unfortunately, while we were given pointer to packet_info at the
>> entry level to our dissector (dissect_libvirt() ->
>> tcp_dissect_pdus() -> dissect_libvirt_message()) it was never
>> propagated to generated/primitive dissectors.
>>
>> But not all dissectors need to allocate memory, so mark the new
>> argument as unused. And while our generator could be rewritten so
>> that the argument is annotated as unused iff it's really unused,
>> I couldn't bother rewriting it. It's generated code after all.
>> Too much work for little gain.
>>
>> Another significant change is that val_to_str() now requires new
>> argument: pointer to allocator to use because it always allocates
>> new memory [2][3].
> 
> IMO the change to propagate the struct needed to replace
> wmem_packet_scope could be separated from the change to the val_to_str
> convertor as it would make the patch a bit more digestable.

Yeah, I thought about that, but the hunk below is basically the only
thing that would be left to do to adapt. So I figured it's not worth the
split.

> 
> Regardless no need to change it now.
> 
> 
>>
>> 1: 
>> https://gitlab.com/wireshark/wireshark/-/commit/5ca5c9ca372e06881b23ba9f4fdcb6b479886444
>> 2: 
>> https://gitlab.com/wireshark/wireshark/-/commit/b63599762468e4cf1783419a5556377604d344bb
>> 3: 
>> https://gitlab.com/wireshark/wireshark/-/commit/84799be215313e61b83a3eaf074f89d6ee349b8c
>> Resolves: https://gitlab.com/libvirt/libvirt/-/issues/823
>> Signed-off-by: Michal Privoznik <[email protected]>
>> ---
>>  tools/wireshark/src/packet-libvirt.c | 157 +++++++++++++++++++--------
>>  tools/wireshark/util/genxdrstub.pl   |  18 +--
>>  2 files changed, 119 insertions(+), 56 deletions(-)
> 
> 
>>  static char *
>> -G_GNUC_PRINTF(3, 0)
>> -vir_val_to_str(const uint32_t val,
>> +G_GNUC_PRINTF(4, 0)
>> +vir_val_to_str(packet_info *pinfo,
>> +               const uint32_t val,
>>                 const value_string *vs,
>>                 const char *fmt)
>>  {
>> -    return val_to_str_wmem(wmem_packet_scope(), val, vs, fmt);
>> +#if WIRESHARK_VERSION < 4006000
>> +    return val_to_str_wmem(pinfo->pool, val, vs, fmt);
>> +#else
>> +    return val_to_str(pinfo->pool, val, vs, fmt);
>> +#endif
>>  }
> 
> The above hunk might need some update based on my query in previous
> patch.
> 

I think this clearly shows that val_to_str_wmem() is called on for
wireshark < 4.6.0.

> Once that is solved:
> 
> Reviewed-by: Peter Krempa <[email protected]>
> 

Thanks,
Michal

Reply via email to