Re: [PATCH] event: fix warning from useless snprintf

[Stephen Hemminger <stephen@networkplumber.org>]

On Sat, 15 Jun 2024 13:43:16 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 24/04/2024 21:10, Stephen Hemminger:
> > On Wed, 24 Apr 2024 17:12:39 +0000
> > "Van Haaren, Harry" <harry.van.haaren@intel.com> wrote:  
> > > From: Stephen Hemminger <stephen@networkplumber.org>  
> > > > "Van Haaren, Harry" <harry.van.haaren@intel.com> wrote:  
> > > > > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > > > >
> > > > > > With Gcc-14, this warning is generated:
> > > > > > ../drivers/event/sw/sw_evdev.c:263:3: warning: 'snprintf' will always be truncated;
> > > > > >     specified size is 12, but format string expands to at least 13 [-Wformat-truncation]
> > > > > >   263 |                 snprintf(buf, sizeof(buf), "sw%d_iq_%d_rob", dev_id, i);
> > > > > >       |                 ^
> > > > > >
> > > > > > Yet the whole printf to the buf is unnecessary. The type string argument
> > > > > > has never been implemented, and should just be NULL.  Removing the
> > > > > > unnecessary snprintf, then means IQ_ROB_NAMESIZE can be removed.    
> > > > >
> > > > > I understand that today the "type" value isn't implemented, but across the DPDK codebase it
> > > > > seems like others are filling in "type" to be some debug-useful name/string. If it was added
> > > > > in future it'd be nice to have the ROB/IQ memory identified by name, like the rest of DPDK components.    
> > > > 
> > > > No, don't bother. This is a case of https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it    
> > > 
> > > I agree that YAGNI perhaps applied when designing the APIs, but the "type" parameter is there now...
> > > Should we add a guidance of "when reworking code, always pass NULL as the type parameter to rte_malloc functions" somewhere in the programmers guide, to align community with this "pass NULL for type" initiative?
> > > 
> > > <snip>
> > > 
> > > Acked-by: Harry van Haaren <harry.van.haaren@intel.com>
> > >   
> > 
> > Did look into Mi-Malloc https://github.com/microsoft/mimalloc
> > it is fast and more complete and good work with huge pages.
> > The way to handle tagging allocations having the library automatically handle it
> > based on the place allocation is called from. Having user do it is not that helpful.  
> 
> But today we have rte_malloc.
> And the type is used in tracing.
> I think having a meaningful name from the caller is not a bad idea.
> 
> 

Ok still useful for tracing, but documentation is incorrect (dump stats doesn't use it), and the
type field doesn't need to be passed as argument to heap code.