On Aug 9, 2016, at 2:12 PM, Andreas Dilger <andreas.dilger@xxxxxxxxx> wrote:
>
> llvm warns about the confusingly written comparison:
>
> !strncmp(argv[i+1], "-", 1) == 0) {
> misc/fsck.c:1178 col 9: warning: logical not is only applied to
> the left hand side of comparison [-Wlogical-not-parentheses]
> misc/fsck.c:1178 col 9: note: add parentheses after the '!' to
> evaluate the comparison first
> misc/fsck.c:1178 col 9: note: add parentheses around left hand
> side expression to silence this warning
>
> It makes sense to simplify this to a character comparison
> rather than using strncmp() to check only one character.
>
> Signed-off-by: Andreas Dilger <andreas.dilger@xxxxxxxxx>
> ---
> misc/fsck.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/misc/fsck.c b/misc/fsck.c
> index 826aaeb..4f918b7 100644
> --- a/misc/fsck.c
> +++ b/misc/fsck.c
> @@ -1174,8 +1174,8 @@ static void PRS(int argc, char *argv[])
> progress_fd = 0;
> else
> goto next_arg;
> - } else if ((i+1) < argc &&
> - !strncmp(argv[i+1], "-", 1) == 0) {
> + } else if (argc > i + 1 &&
> + argv[i + 1][0] == '-') {
> progress_fd = string_to_int(argv[i]);
> if (progress_fd < 0)
> progress_fd = 0;
Note that it isn't clear whether the original logic also contained a bug,
with both "!strncmp()" and the comparison with "== 0". At first glance
it appeared that this was a bug to both negate and compare with 0, but
in further review I think that this should _not_ parse negative numbers
and use "-" as the fd. Unfortunately, it isn't documented what "-" means.
I'll push a v2 patch that keeps the original logic, and Ted can choose
which one is correct.
Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP using GPGMail
