Messages in this thread | | | From | Packet Geek <> | Date | Wed, 31 Jan 2024 20:45:56 -0800 | Subject | Re: [PATCH net-next v2 3/8] net: netconsole: move newline trimming to function |
| |
On Tue, Jan 30, 2024 at 1:16 AM Breno Leitao <leitao@debian.org> wrote: > > On Fri, Jan 26, 2024 at 03:13:38PM -0800, Matthew Wood wrote: > > Move newline trimming logic from `dev_name_store()` to a new function > > (trim_newline()) for shared use in netconsole.c > > > > Signed-off-by: Matthew Wood <thepacketgeek@gmail.com> > > --- > > drivers/net/netconsole.c | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c > > index 085350beca87..b280d06bf152 100644 > > --- a/drivers/net/netconsole.c > > +++ b/drivers/net/netconsole.c > > @@ -230,6 +230,16 @@ static struct netconsole_target *to_target(struct config_item *item) > > struct netconsole_target, group); > > } > > > > +/* Get rid of possible trailing newline, returning the new length */ > > +static void trim_newline(char *s, size_t maxlen) > > +{ > > + size_t len; > > + > > + len = strnlen(s, maxlen); > > + if (s[len - 1] == '\n') > > + s[len - 1] = '\0'; > > +} > > I am thinking about this one. Should we replace the first `\n` in the > file by `\0` no matter where it is? This will probably make it easier to > implement the netconsd, where we know it will be impossible to have `\n` > in the userdata. > > Maybe something as: > > static inline void trim_newline(char *str) > { > char *pos = strchr(str, '\n'); > > if (pos) > *pos = '\0'; > } > > > All in all, this is a good clean up, which make the code easier to read. > Thanks!
I like this idea, I agree that only accepting userdata values upto the first newline clears up the expectations for log output and parsing on the receiving side. I would prefer that to the case where multiple values (delimited by newlines) are somehow attempted with a single key, seems like just using additional key/value pairs would be cleaner.
| |