[PATCH] event: fix warning from useless snprintf

Stephen Hemminger stephen at networkplumber.org
Sat Jun 15 18:04:42 CEST 2024


On Sat, 15 Jun 2024 13:43:16 +0200
Thomas Monjalon <thomas at 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 at intel.com> wrote:  
> > > From: Stephen Hemminger <stephen at networkplumber.org>  
> > > > "Van Haaren, Harry" <harry.van.haaren at intel.com> wrote:  
> > > > > > From: Stephen Hemminger <stephen at 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 at 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.


More information about the dev mailing list