On Sat, 15 Jun 2024 13:43:16 +0200 Thomas Monjalon <tho...@monjalon.net> wrote:
> 24/04/2024 21:10, Stephen Hemminger: > > On Wed, 24 Apr 2024 17:12:39 +0000 > > "Van Haaren, Harry" <harry.van.haa...@intel.com> wrote: > > > From: Stephen Hemminger <step...@networkplumber.org> > > > > "Van Haaren, Harry" <harry.van.haa...@intel.com> wrote: > > > > > > From: Stephen Hemminger <step...@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.haa...@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.
