| Message ID | 20240903140757.2802765-3-leitao@debian.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | netconsole refactoring and warning fix | expand |
On Tue, Sep 03, 2024 at 07:07:45AM -0700, Breno Leitao wrote: > The send_ext_msg_udp() function has become quite large, currently > spanning 102 lines. Its complexity, along with extensive pointer and > offset manipulation, makes it difficult to read and error-prone. > > The function has evolved over time, and it’s now due for a refactor. > > To improve readability and maintainability, isolate the case where no > message fragmentation occurs into a separate function, into a new > send_msg_no_fragmentation() function. This scenario covers about 95% of > the messages. > > Signed-off-by: Breno Leitao <leitao@debian.org> Thanks, The nit below aside this looks good to me. Reviewed-by: Simon Horman <horms@kernel.org> > @@ -1090,23 +1116,8 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg, > release_len = strlen(release) + 1; > } > > - if (msg_len + release_len + userdata_len <= MAX_PRINT_CHUNK) { > - /* No fragmentation needed */ > - if (nt->release) { > - scnprintf(buf, MAX_PRINT_CHUNK, "%s,%s", release, msg); > - msg_len += release_len; > - } else { > - memcpy(buf, msg, msg_len); > - } > - > - if (userdata) > - msg_len += scnprintf(&buf[msg_len], > - MAX_PRINT_CHUNK - msg_len, > - "%s", userdata); > - > - netpoll_send_udp(&nt->np, buf, msg_len); > - return; > - } > + if (msg_len + release_len + userdata_len <= MAX_PRINT_CHUNK) > + return send_msg_no_fragmentation(nt, msg, userdata, msg_len, release_len); nit: This appears to be fixed in the following patch, but the above line could be wrapped here. > > /* need to insert extra header fields, detect header and body */ > header = msg; > -- > 2.43.5 >
diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c index 03150e513cb2..0e43dacbd291 100644 --- a/drivers/net/netconsole.c +++ b/drivers/net/netconsole.c @@ -1058,6 +1058,32 @@ static struct notifier_block netconsole_netdev_notifier = { .notifier_call = netconsole_netdev_event, }; +static void send_msg_no_fragmentation(struct netconsole_target *nt, + const char *msg, + const char *userdata, + int msg_len, + int release_len) +{ + static char buf[MAX_PRINT_CHUNK]; /* protected by target_list_lock */ + const char *release; + + if (release_len) { + release = init_utsname()->release; + + scnprintf(buf, MAX_PRINT_CHUNK, "%s,%s", release, msg); + msg_len += release_len; + } else { + memcpy(buf, msg, msg_len); + } + + if (userdata) + msg_len += scnprintf(&buf[msg_len], + MAX_PRINT_CHUNK - msg_len, + "%s", userdata); + + netpoll_send_udp(&nt->np, buf, msg_len); +} + /** * send_ext_msg_udp - send extended log message to target * @nt: target to send message to @@ -1090,23 +1116,8 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg, release_len = strlen(release) + 1; } - if (msg_len + release_len + userdata_len <= MAX_PRINT_CHUNK) { - /* No fragmentation needed */ - if (nt->release) { - scnprintf(buf, MAX_PRINT_CHUNK, "%s,%s", release, msg); - msg_len += release_len; - } else { - memcpy(buf, msg, msg_len); - } - - if (userdata) - msg_len += scnprintf(&buf[msg_len], - MAX_PRINT_CHUNK - msg_len, - "%s", userdata); - - netpoll_send_udp(&nt->np, buf, msg_len); - return; - } + if (msg_len + release_len + userdata_len <= MAX_PRINT_CHUNK) + return send_msg_no_fragmentation(nt, msg, userdata, msg_len, release_len); /* need to insert extra header fields, detect header and body */ header = msg;
The send_ext_msg_udp() function has become quite large, currently spanning 102 lines. Its complexity, along with extensive pointer and offset manipulation, makes it difficult to read and error-prone. The function has evolved over time, and it’s now due for a refactor. To improve readability and maintainability, isolate the case where no message fragmentation occurs into a separate function, into a new send_msg_no_fragmentation() function. This scenario covers about 95% of the messages. Signed-off-by: Breno Leitao <leitao@debian.org> --- drivers/net/netconsole.c | 45 +++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 17 deletions(-)