From b4cfcde4db8f5787c6c8a3912b0f2667becd1995 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 01:22:59 -0400 Subject: [PATCH 1/4] rev-list: handle flags for --indexed-objects When a traversal sees the --indexed-objects option, it adds all blobs and valid cache-trees from the index to the traversal using add_index_objects_to_pending(). But that function totally ignores its flags parameter! That means that doing: git rev-list --objects --indexed-objects and git rev-list --objects --not --indexed-objects produce the same output, because we ignore the UNINTERESTING flag when walking the index in the second example. Nobody noticed because this feature was added as a way for tools like repack to increase their coverage of reachable objects, meaning it would only be used like the first example above. But since it's user facing (and because the documentation describes it "as if the objects are listed on the command line"), we should make sure the negative case behaves sensibly. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- revision.c | 15 +++++++++------ t/t6000-rev-list-misc.sh | 7 +++++++ 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/revision.c b/revision.c index a1ddb9e11c..8e56c9641f 100644 --- a/revision.c +++ b/revision.c @@ -1321,13 +1321,14 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags) } static void add_cache_tree(struct cache_tree *it, struct rev_info *revs, - struct strbuf *path) + struct strbuf *path, unsigned int flags) { size_t baselen = path->len; int i; if (it->entry_count >= 0) { struct tree *tree = lookup_tree(revs->repo, &it->oid); + tree->object.flags |= flags; add_pending_object_with_path(revs, &tree->object, "", 040000, path->buf); } @@ -1335,14 +1336,15 @@ static void add_cache_tree(struct cache_tree *it, struct rev_info *revs, for (i = 0; i < it->subtree_nr; i++) { struct cache_tree_sub *sub = it->down[i]; strbuf_addf(path, "%s%s", baselen ? "/" : "", sub->name); - add_cache_tree(sub->cache_tree, revs, path); + add_cache_tree(sub->cache_tree, revs, path, flags); strbuf_setlen(path, baselen); } } static void do_add_index_objects_to_pending(struct rev_info *revs, - struct index_state *istate) + struct index_state *istate, + unsigned int flags) { int i; @@ -1356,13 +1358,14 @@ static void do_add_index_objects_to_pending(struct rev_info *revs, blob = lookup_blob(revs->repo, &ce->oid); if (!blob) die("unable to add index blob to traversal"); + blob->object.flags |= flags; add_pending_object_with_path(revs, &blob->object, "", ce->ce_mode, ce->name); } if (istate->cache_tree) { struct strbuf path = STRBUF_INIT; - add_cache_tree(istate->cache_tree, revs, &path); + add_cache_tree(istate->cache_tree, revs, &path, flags); strbuf_release(&path); } } @@ -1372,7 +1375,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags) struct worktree **worktrees, **p; read_index(revs->repo->index); - do_add_index_objects_to_pending(revs, revs->repo->index); + do_add_index_objects_to_pending(revs, revs->repo->index, flags); if (revs->single_worktree) return; @@ -1388,7 +1391,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags) if (read_index_from(&istate, worktree_git_path(wt, "index"), get_worktree_git_dir(wt)) > 0) - do_add_index_objects_to_pending(revs, &istate); + do_add_index_objects_to_pending(revs, &istate, flags); discard_index(&istate); } free_worktrees(worktrees); diff --git a/t/t6000-rev-list-misc.sh b/t/t6000-rev-list-misc.sh index fb4d295aa0..0507999729 100755 --- a/t/t6000-rev-list-misc.sh +++ b/t/t6000-rev-list-misc.sh @@ -90,11 +90,18 @@ test_expect_success 'rev-list can show index objects' ' 9200b628cf9dc883a85a7abc8d6e6730baee589c two EOF echo only-in-index >only-in-index && + test_when_finished "git reset --hard" && git add only-in-index && git rev-list --objects --indexed-objects >actual && test_cmp expect actual ' +test_expect_success 'rev-list can negate index objects' ' + git rev-parse HEAD >expect && + git rev-list -1 --objects HEAD --not --indexed-objects >actual && + test_cmp expect actual +' + test_expect_success '--bisect and --first-parent can not be combined' ' test_must_fail git rev-list --bisect --first-parent HEAD ' From c27cc94fad36ba46fea4a031d6df9d45b931f421 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 01:23:09 -0400 Subject: [PATCH 2/4] approxidate: handle pending number for "specials" The approxidate parser has a table of special keywords like "yesterday", "noon", "pm", etc. Some of these, like "pm", do the right thing if we've recently seen a number: "3pm" is what you'd think. However, most of them do not look at or modify the pending-number flag at all, which means a number may "jump" across a significant keyword and be used unexpectedly. For example, when parsing: January 5th noon pm we'd connect the "5" to "pm", and ignore it as a day-of-month. This is obviously a bit silly, as "noon" already implies "pm". And other mis-parsed things are generally as silly ("January 5th noon, years ago" would connect the 5 to "years", but probably nobody would type that). However, the fix is simple: when we see a keyword like "noon", we should flush the pending number (as we would if we hit another number, or the end of the string). In a few of the specials that actually modify the day, we can simply throw away the number (saying "Jan 5 yesterday" should not respect the number at all). Note that we have to either move or forward-declare the static pending_number() to make it accessible to these functions; this patch moves it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- date.c | 60 +++++++++++++++++++++++++++---------------------- t/t0006-date.sh | 1 + 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/date.c b/date.c index 49f943e25b..7adce327a3 100644 --- a/date.c +++ b/date.c @@ -887,13 +887,42 @@ static time_t update_tm(struct tm *tm, struct tm *now, time_t sec) return n; } +/* + * Do we have a pending number at the end, or when + * we see a new one? Let's assume it's a month day, + * as in "Dec 6, 1992" + */ +static void pending_number(struct tm *tm, int *num) +{ + int number = *num; + + if (number) { + *num = 0; + if (tm->tm_mday < 0 && number < 32) + tm->tm_mday = number; + else if (tm->tm_mon < 0 && number < 13) + tm->tm_mon = number-1; + else if (tm->tm_year < 0) { + if (number > 1969 && number < 2100) + tm->tm_year = number - 1900; + else if (number > 69 && number < 100) + tm->tm_year = number; + else if (number < 38) + tm->tm_year = 100 + number; + /* We screw up for number = 00 ? */ + } + } +} + static void date_now(struct tm *tm, struct tm *now, int *num) { + *num = 0; update_tm(tm, now, 0); } static void date_yesterday(struct tm *tm, struct tm *now, int *num) { + *num = 0; update_tm(tm, now, 24*60*60); } @@ -908,16 +937,19 @@ static void date_time(struct tm *tm, struct tm *now, int hour) static void date_midnight(struct tm *tm, struct tm *now, int *num) { + pending_number(tm, num); date_time(tm, now, 0); } static void date_noon(struct tm *tm, struct tm *now, int *num) { + pending_number(tm, num); date_time(tm, now, 12); } static void date_tea(struct tm *tm, struct tm *now, int *num) { + pending_number(tm, num); date_time(tm, now, 17); } @@ -953,6 +985,7 @@ static void date_never(struct tm *tm, struct tm *now, int *num) { time_t n = 0; localtime_r(&n, tm); + *num = 0; } static const struct special { @@ -1110,33 +1143,6 @@ static const char *approxidate_digit(const char *date, struct tm *tm, int *num, return end; } -/* - * Do we have a pending number at the end, or when - * we see a new one? Let's assume it's a month day, - * as in "Dec 6, 1992" - */ -static void pending_number(struct tm *tm, int *num) -{ - int number = *num; - - if (number) { - *num = 0; - if (tm->tm_mday < 0 && number < 32) - tm->tm_mday = number; - else if (tm->tm_mon < 0 && number < 13) - tm->tm_mon = number-1; - else if (tm->tm_year < 0) { - if (number > 1969 && number < 2100) - tm->tm_year = number - 1900; - else if (number > 69 && number < 100) - tm->tm_year = number; - else if (number < 38) - tm->tm_year = 100 + number; - /* We screw up for number = 00 ? */ - } - } -} - static timestamp_t approxidate_str(const char *date, const struct timeval *tv, int *error_ret) diff --git a/t/t0006-date.sh b/t/t0006-date.sh index 64ff86df8e..b7ea5fbc36 100755 --- a/t/t0006-date.sh +++ b/t/t0006-date.sh @@ -113,6 +113,7 @@ check_approxidate '3:00' '2009-08-30 03:00:00' check_approxidate '15:00' '2009-08-30 15:00:00' check_approxidate 'noon today' '2009-08-30 12:00:00' check_approxidate 'noon yesterday' '2009-08-29 12:00:00' +check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' check_approxidate 'last tuesday' '2009-08-25 19:20:00' check_approxidate 'July 5th' '2009-07-05 19:20:00' From 8a2c174677e62fe778f5616bf67097cad3a5636e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 2 Nov 2018 01:23:22 -0400 Subject: [PATCH 3/4] pathspec: handle non-terminated strings with :(attr) The pathspec code always takes names to be matched as a name/namelen pair, but match_attrs() never looks at namelen, and just treats "name" like a NUL-terminated string, passing it to git_check_attr(). This usually works anyway. Every caller passes a NUL-terminated string, and in all but one the passed-in length is the same as the length of the string (the exception is dir_path_match(), which may pass a smaller length to drop a trailing slash). So we won't currently ever read random memory, and the one case I found actually happens to work correctly because the attr code can handle the trailing slash itself. But it's still worth addressing, as the function interface implies that the name does not have to be NUL-terminated, making this an accident waiting to happen. Since teaching git_check_attr() to take a ptr/len pair would be a big refactor, we'll just allocate a new string. We can do this only when necessary, which avoids paying the cost for most callers. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- dir.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/dir.c b/dir.c index 47c2fca8dc..ab6477d777 100644 --- a/dir.c +++ b/dir.c @@ -281,8 +281,15 @@ static int match_attrs(const struct index_state *istate, const struct pathspec_item *item) { int i; + char *to_free = NULL; + + if (name[namelen]) + name = to_free = xmemdupz(name, namelen); git_check_attr(istate, name, item->attr_check); + + free(to_free); + for (i = 0; i < item->attr_match_nr; i++) { const char *value; int matched; From aa097b88e91bd748c4508b042e618e8292e15ad5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 6 Nov 2018 20:12:53 -0500 Subject: [PATCH 4/4] approxidate: fix NULL dereference in date_time() When we see a time like "noon", we pass "12" to our date_time() helper, which sets the hour to 12pm. If the current time is before noon, then we wrap around to yesterday using date_yesterday(). But unlike the normal calls to date_yesterday() from approxidate_alpha(), we pass a NULL "num" parameter. Since c27cc94fad (approxidate: handle pending number for "specials", 2018-11-02), that causes a segfault. One way to fix this is by checking for NULL. But arguably date_time() is abusing our helper by passing NULL in the first place (and this is the only case where one of these "special" parsers is used this way). So instead, let's have it just do the 1-day subtraction itself. It's still just a one-liner due to our update_tm() helper. Note that the test added here is a little funny, as we say "10am noon", which makes the "10am" seem pointless. But this bug can only be triggered when it the currently-parsed hour is before the special time. The latest special time is "tea" at 1700, but t0006 uses a hard-coded TEST_DATE_NOW of 1900. We could reset TEST_DATE_NOW, but that may lead to confusion in other tests. Just saying "10am noon" makes this test self-contained. Reported-by: Carlo Arenas Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- date.c | 2 +- t/t0006-date.sh | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/date.c b/date.c index 7adce327a3..9bc15df6f9 100644 --- a/date.c +++ b/date.c @@ -929,7 +929,7 @@ static void date_yesterday(struct tm *tm, struct tm *now, int *num) static void date_time(struct tm *tm, struct tm *now, int hour) { if (tm->tm_hour < hour) - date_yesterday(tm, now, NULL); + update_tm(tm, now, 24*60*60); tm->tm_hour = hour; tm->tm_min = 0; tm->tm_sec = 0; diff --git a/t/t0006-date.sh b/t/t0006-date.sh index b7ea5fbc36..ffb2975e48 100755 --- a/t/t0006-date.sh +++ b/t/t0006-date.sh @@ -114,6 +114,7 @@ check_approxidate '15:00' '2009-08-30 15:00:00' check_approxidate 'noon today' '2009-08-30 12:00:00' check_approxidate 'noon yesterday' '2009-08-29 12:00:00' check_approxidate 'January 5th noon pm' '2009-01-05 12:00:00' +check_approxidate '10am noon' '2009-08-29 12:00:00' check_approxidate 'last tuesday' '2009-08-25 19:20:00' check_approxidate 'July 5th' '2009-07-05 19:20:00'