On Tue, Oct 14, 2025 at 14:45:03 +0200, Michal Prívozník wrote: > On 10/14/25 14:23, Peter Krempa wrote: > > On Tue, Oct 14, 2025 at 08:31:46 +0200, Michal Privoznik via Devel wrote: > >> From: Michal Privoznik <[email protected]> > >> > >> One of the problems of using val_to_str() is that it may return a > >> const string from given table ('vs'), OR return an allocated one. > >> Since the caller has no idea which case it is, it resides to safe > >> option and don't free returned string. But that might lead to a > >> memleak. This behaviour is fixed with wireshark-4.6.0 and support > >> for it will be introduced soon. But first, make vir_val_to_str() > >> behave like fixed val_to_str() from newer wireshark: just always > >> allocate the string. > >> > >> Now, if val_to_str() needs to allocate new memory it obtains > >> allocator by calling wmem_packet_scope() which is what we may do > >> too. > >> > >> Hand in hand with that, we need to free the memory using the > >> correct allocator, hence wmem_free(). But let's put it into a > >> wrapper vir_wmem_free() because just like val_to_str(), it'll > >> need additional argument when adapting to new wireshark. > >> > >> Oh, and freeing the memory right after col_add_fstr() is safe as > >> it uses vsnprintf() under the hood to format passed args. > >> > >> One last thing, the wmem.h file used to live under epan/wmem/ but > >> then in v3.5.0~240 [1] was moved to wsutil/wmem/. > >> > >> 1: > >> https://gitlab.com/wireshark/wireshark/-/commit/7f9c1f5f92c131354fc8b2b88d473706786064c0 > >> Signed-off-by: Michal Privoznik <[email protected]> > >> --- > >> meson.build | 20 ++++++++++++++++ > >> tools/wireshark/src/meson.build | 1 + > >> tools/wireshark/src/packet-libvirt.c | 35 ++++++++++++++++++++++------ > >> 3 files changed, 49 insertions(+), 7 deletions(-) > > > > [...] > > > >> @@ -140,13 +145,19 @@ static const value_string status_strings[] = { > >> { -1, NULL } > >> }; > >> > >> -static const char * > >> +static char * > >> G_GNUC_PRINTF(3, 0) > >> vir_val_to_str(const uint32_t val, > >> const value_string *vs, > >> const char *fmt) > >> { > >> - return val_to_str(val, vs, fmt); > >> + return val_to_str_wmem(wmem_packet_scope(), val, vs, fmt); > > > > I didn't try building this but grepping in the wireshark code base shows > > only: > > > > wsutil/value_string.h:rval_to_str_wmem(wmem_allocator_t* scope, const > > uint32_t val, const range_string *rs, const char *fmt) > > wsutil/value_string.h:bytesval_to_str_wmem(wmem_allocator_t* scope, const > > uint8_t *val, const size_t val_len, const bytes_string *bs, const char *fmt) > > > > Mind you, val_to_str_wmem() is available only in pre-4.6.0 era. > > https://gitlab.com/wireshark/wireshark/-/commit/84799be215313e61b83a3eaf074f89d6ee349b8c > > wireshark.git $ git describe --contains > 84799be215313e61b83a3eaf074f89d6ee349b8c > v4.6.0rc0~120 > > In this commit val_to_str_wmem() was renamed to val_to_str(). The whole > point of these patches up to this one is to prepare for this change.
Ah I didn't realize they've renamed in completely and didn't bother checking history because I had only a --depth 1 clone. Reviewed-by: Peter Krempa <[email protected]>
