On Wed, Jun 19, 2024 at 10:25:17AM +0200, Stefano Brivio wrote:On Wed, 19 Jun 2024 12:11:51 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Eh, ok. You more or less convinced me. -- 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/~dgibsonOn Tue, Jun 18, 2024 at 08:02:16AM +0200, Stefano Brivio wrote:Well, it depends. If you're used to perror() it's obvious where the error description will appear, and it's actually faster for me to read something called "_perror" than %s plus the argument. Plus we can save a few lines like that and substantially improve readability in some cases: if (prctl(PR_CAPBSET_DROP, i, 0, 0, 0) && errno != EINVAL && errno != EPERM) - die("Couldn't drop cap %i from bounding set: %s", - i, strerror(errno)); + die_perror("Couldn't drop cap %i from bounding set", i);On Tue, 18 Jun 2024 10:46:36 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote: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.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 <sbrivio(a)redhat.com> 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.