On Wed, May 4, 2016 at 12:40 AM, Bernhard Voelker <m...@bernhard-voelker.de> wrote: > On 05/04/2016 05:59 AM, Jim Meyering wrote: >> - bool default_format = (just_user + just_group + just_group_list >> + bool default_format = (0U + just_user + just_group + just_group_list >> + just_context == 0); > > These are all bool - wouldn't it be better to use boolean instead of > arithmetical operators?
Thanks for the review. Indeed. Using "0U" there is a bit too much: a hack upon the hack of using addition. While using addition can be seen as slightly more readable (assuming you know the idiom), or even better because the generated code is jump-free, I would argue that this code should be readable, and that whether there is a short-circuiting jump is irrelevant to the performance of id. While it is tempting to use "|" (the code generated by gcc-5.3 -O3 is smaller and still jump-free: https://godbolt.org/g/zqMTtg), that still feels dubious, especially when you remember that with gnulib, we may still be simulating "bool" on some crufty systems. So it seems best to use the bool-appropriate operators. Here's the adjusted patch:
From 9a4df07016827fe130016b1c3a4adaf91c54c301 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyer...@fb.com> Date: Tue, 3 May 2016 20:56:20 -0700 Subject: [PATCH] maint: avoid new warning from gcc (GCC) 7.0.0 20160503 (experimental) * src/id.c (main): When configured with --enable-gcc-warnings and using the very latest gcc built from git, building would fail with this: src/id.c:200:8: error: assuming signed overflow does not occur when \ simplifying conditional to constant [-Werror=strict-overflow] bool default_format = (just_user + just_group + just_group_list ^~~~~~~~~~~~~~ Rewrite to use bool-appropriate operators. --- src/id.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/id.c b/src/id.c index 38844af..218ee5a 100644 --- a/src/id.c +++ b/src/id.c @@ -197,8 +197,10 @@ main (int argc, char **argv) if (just_user + just_group + just_group_list + just_context > 1) error (EXIT_FAILURE, 0, _("cannot print \"only\" of more than one choice")); - bool default_format = (just_user + just_group + just_group_list - + just_context == 0); + bool default_format = ! (just_user + || just_group + || just_group_list + || just_context); if (default_format && (use_real || use_name)) error (EXIT_FAILURE, 0, -- 2.8.0-rc2