Re: [PATCH 03/11] log: Drop newlines in the middle of the perror()-like messages

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]

On Thu, Jul 25, 2024 at 01:27:09PM +0200, Stefano Brivio wrote:
> On Thu, 25 Jul 2024 13:26:47 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > On Wed, Jul 24, 2024 at 11:50:09PM +0200, Stefano Brivio wrote:
> > > Calling vlogmsg() twice from logmsg_perror() results in this beauty:
> > > 
> > >   $ ./pasta -i foo
> > >   Invalid interface name foo
> > >   : No such device
> > > 
> > > because the first part of the message, corresponding to the first
> > > call, doesn't end with a newline, and vlogmsg() adds it.
> > > 
> > > Given that we can't easily append an argument (error description) to
> > > a variadic list, add a 'newline' parameter to all the functions that
> > > currently add a newline if missing, and disable that on the first call
> > > to vlogmsg() from logmsg_perror(). Not very pretty but I can't think
> > > of any solution that's less messy than this.
> > > 
> > > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> > 
> > I think my personal inclination would be to rename all the
> > lowest-level functions slightly and remove the newline adding logic
> > unconditionally.  The create wrappers under the old name which add the
> > "\n".  I think that can be done in an easy macro, since the "\n" can
> > be constant string appended to the format string.  Just the special
> > paths that need to suppress the newline would call the low level "no
> > newline" variants.
> 
> I gave it a try, but the problem is that the "easy macro" needs to be
> conditional, depending on whether the newline is there or not: we don't
> want to add a newline in case somebody already added it by mistake (or
> habit).

Well, I mean we could just add the newline and consider it a bug if
someone puts one in themselves.

> That could probably be done as well with an intermediate function, but
> it's getting a bit too complicated (at least for me right now).
> 
> > But, I don't actually care that much so
> > 
> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]