Pierre Habouzit <madcoder@xxxxxxxxxx> writes:
> Except that you should *here* test if count > 3, because if you take 2
> out of 3, you will then call grep with exactly 1 argument. This can only
> happen if the user had sth like MAXARGS - 3 (or - 4, it does not really
> matter for the explanation anyway) options, then he will have wrong
> results. Though, as MAXARGS is _very_ long, that corresponds to insane
> situations that we just want to die() for ;)
Yes, that is exactly why there is die in if (count < 2 && kept)
case, but I think you are right. That should be if (count <=
2), because if we say "Oh we have two and we are happy" and
later say "and we hold these two back", then we will end up
driving grep with 0 paths which is not what we want.
So if we change the test to (count <= 2), then:
- while we are still iterating over the cache and MAXARGS is
reached, we call flush() with kept != NULL.
- If count is too small that means the user gave insanely
large number of options and we die;
- Otherwise, we have verified we have more than two. We send
everything but the last two, and keep 2.
- after finishing to iterate over the cache, we call flush()
with kept == NULL.
- If count is small, then we may add /dev/null; otherwise we
do not have to (the code would add it when count == 2 if we
change the test as you suggested, which is not necessary,
but would not hurt);
- Otherwise we have more than 1 which means we do not have to
worry about /dev/null at all.
So this fix on top of the patch you are commenting on would be
sufficient, I guess...
builtin-grep.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin-grep.c b/builtin-grep.c
index 4ef30ea..c7b45c4 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -202,7 +202,7 @@ static int flush_grep(struct grep_opt *opt,
int count = argc - arg0;
const char *kept_0 = NULL;
- if (count < 2) {
+ if (count <= 2) {
/*
* Because we keep at least 2 paths in the call from
* the main loop (i.e. kept != NULL), and MAXARGS is
@@ -221,7 +221,8 @@ static int flush_grep(struct grep_opt *opt,
* get "-H" behaviour of GNU grep portably but when we
* are not doing "-l" nor "-L" nor "-c".
*/
- if (!opt->name_only &&
+ if (count == 1 &&
+ !opt->name_only &&
!opt->unmatch_name_only &&
!opt->count) {
argv[argc++] = "/dev/null";
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
