Thread (5 messages) 5 messages, 3 authors, 2016-08-10

Re: [PATCH] fsck: fix strange logic

From: Andreas Dilger <hidden>
Date: 2016-08-09 20:26:25

On Aug 9, 2016, at 2:12 PM, Andreas Dilger [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
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




Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help