Re: [PATCH 4/6] log: Add _perror() logging function variants

On Tue, Jun 18, 2024 at 08:02:16AM +0200, Stefano Brivio wrote:

On Tue, 18 Jun 2024 10:46:36 +1000 David Gibson wrote:

...

On Mon, Jun 17, 2024 at 02:03:17PM +0200, Stefano Brivio wrote:

...

In many places, we have direct perror() calls, which completely bypass logging functions and log files.

They are definitely convenient: offer similar convenience with _perror() logging variants, so that we can drop those direct perror() calls.

Signed-off-by: Stefano Brivio

Hm, for anything bigger than like a screenful of code, I generally find an explicit message with strerror(errno) more useful than perror() or equivalents, but I guess if you think these are useful.

Okay, yes, it probably makes sense to have more descriptive messages as you suggest in the comment to 5/6, but even then, we still have a lot of cases like this one (from 6/6):

- warn("lseek() failed on /proc/net file: %s", strerror(errno)); + warn_perror("lseek() failed on /proc/net file");

where these _perror() variants make for tidier code, I find, regardless of the error message itself.

Eh, I mildly prefer the first variant. It is slightly longer, but makes it very clear where the strerror piece is going to appear in the context of the whole message. It's not a strong preference, though. -- 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