On Mon, Aug 06, 2018 at 10:44:14AM +0200, Christian Couder wrote:
> Taking a look at how we use regexec() in our code base, it looks like
> it might be better to use regexec_buf() defined in
> "git-compat-util.h".
>
> I am not completely sure about that because apply.c has:
>
> status = regexec(stamp, timestamp, ARRAY_SIZE(m), m, 0);
> if (status) {
> if (status != REG_NOMATCH)
> warning(_("regexec returned %d for input: %s"),
> status, timestamp);
> return 0;
> }
>
> Though the above uses a regex that is defined in apply.c. The regex
> doesn't come from the config file.
>
> Actually except the above there is a mix of regexec() and
> regexec_buf() in our code base, which are used with only 0, 1 or 2
> capture groups, so it is not very clear what should be used.
I don't think we need regexec_buf(). The advantage it has is that it can
operate on strings that aren't NUL-terminated, but that isn't the case
here.
> And anyway I still don't see how we could diagnose when the end user
> input requires more captures than we support.
We can use the final element as a sentinel, and complain if it gets
filled in:
diff --git a/delta-islands.c b/delta-islands.c
index dcc6590cc1..18426ffb18 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -375,6 +375,10 @@ static int find_island_for_ref(const char *refname, const struct object_id *oid,
if (i < 0)
return 0;
+ if (matches[ARRAY_SIZE(matches)-1].rm_so != -1)
+ die("island regex had too many matches (max=%d)",
+ (int)ARRAY_SIZE(matches) - 2);
+
for (m = 1; m < ARRAY_SIZE(matches); m++) {
regmatch_t *match = &matches[m];
The big downside is that it only kicks in when you actually successfully
make a match. So you could have:
[pack]
island = refs/(one)/(two)/(three)/(four)/(five)/(six)/(seven)
in your config for years, and then one day it blows up when somebody
actually has a ref that matches it.
I think it would be fine to just say "we only respect the first N
capture groups". And maybe even issue a warning (based on the detection
above). I'd also be fine with bumping the "matches" array to something
more ridiculous, like 32. The current value of 8 was supposed to be
ridiculous already (we've never used more than 2).
-Peff
