> From: Stephen Hemminger <step...@networkplumber.org> > Sent: Wednesday, April 24, 2024 4:45 AM > To: dev@dpdk.org > Cc: Richardson, Bruce; Stephen Hemminger; Van Haaren, Harry; Jerin Jacob > Subject: [PATCH] event: fix warning from useless snprintf > > 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. > Fixes: 5ffb2f142d95 ("event/sw: support event queues") > > Signed-off-by: Stephen Hemminger <step...@networkplumber.org> > --- > drivers/event/sw/iq_chunk.h | 2 -- > drivers/event/sw/sw_evdev.c | 5 +---- > 2 files changed, 1 insertion(+), 6 deletions(-) > > diff --git a/drivers/event/sw/iq_chunk.h b/drivers/event/sw/iq_chunk.h > index 7a7a8782e6..e638142dbc 100644 > --- a/drivers/event/sw/iq_chunk.h > +++ b/drivers/event/sw/iq_chunk.h > @@ -9,8 +9,6 @@ > #include <stdbool.h> > #include <rte_eventdev.h> > > -#define IQ_ROB_NAMESIZE 12 We can over-provision the temporary buffer, and solve the problem with minimal changes; +#define IQ_ROB_NAMESIZE 64 > - > struct __rte_cache_aligned sw_queue_chunk { > struct rte_event events[SW_EVS_PER_Q_CHUNK]; > struct sw_queue_chunk *next; > diff --git a/drivers/event/sw/sw_evdev.c b/drivers/event/sw/sw_evdev.c > index 1c01b069fe..19a52afc7d 100644 > --- a/drivers/event/sw/sw_evdev.c > +++ b/drivers/event/sw/sw_evdev.c > @@ -228,9 +228,7 @@ qid_init(struct sw_evdev *sw, unsigned int idx, int type, > const struct rte_event_queue_conf *queue_conf) > { > unsigned int i; > - int dev_id = sw->data->dev_id; > int socket_id = sw->data->socket_id; > - char buf[IQ_ROB_NAMESIZE]; > struct sw_qid *qid = &sw->qids[idx]; > > /* Initialize the FID structures to no pinning (-1), and zero packets > */ > @@ -260,8 +258,7 @@ qid_init(struct sw_evdev *sw, unsigned int idx, int type, > goto cleanup; > } > > - snprintf(buf, sizeof(buf), "sw%d_iq_%d_rob", dev_id, i); There is a 2nd hidden bug here; the "i" variable iterates the ROB size, and does not represent the QID index. Changing the "i" to "idx" solves, and prints the expected output instead: snprintf(buf, sizeof(buf), "sw%d_iq_%d_rob", dev_id, idx); > - qid->reorder_buffer = rte_zmalloc_socket(buf, > + qid->reorder_buffer = rte_zmalloc_socket(NULL, > window_size * sizeof(qid->reorder_buffer[0]), > 0, socket_id); > if (!qid->reorder_buffer) { > -- > 2.43.0 I'm happy to send a patch with the above, if that seems a good solution to you Stephen? Regards, -Harry