| Message ID | 20190722181923.21572-1-dev+git@drbeat.li (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | grep: print the pcre2_jit_on value | expand |
Beat Bolli <dev+git@drbeat.li> writes: > When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value > of pcre1_jit_on. > > Print the value of pcre2_jit_on instead. > > Signed-off-by: Beat Bolli <dev+git@drbeat.li> > --- Thanks. > grep.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/grep.c b/grep.c > index f7c3a5803e..cd952ef5d3 100644 > --- a/grep.c > +++ b/grep.c > @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt > pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack); > } else if (p->pcre2_jit_on != 0) { > BUG("The pcre2_jit_on variable should be 0 or 1, not %d", > - p->pcre1_jit_on); > + p->pcre2_jit_on); > } > }
Hi Beat, On Mon, 22 Jul 2019, Beat Bolli wrote: > When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value > of pcre1_jit_on. > > Print the value of pcre2_jit_on instead. > > Signed-off-by: Beat Bolli <dev+git@drbeat.li> > --- > grep.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/grep.c b/grep.c > index f7c3a5803e..cd952ef5d3 100644 > --- a/grep.c > +++ b/grep.c > @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt > pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack); > } else if (p->pcre2_jit_on != 0) { > BUG("The pcre2_jit_on variable should be 0 or 1, not %d", > - p->pcre1_jit_on); > + p->pcre2_jit_on); Seems obviously good. Maybe while you're in the vicinity, you can add that information to the `--debug` output? Thanks, Dscho > } > } > > -- > 2.21.0.1020.gf2820cf01a > >
Hi Dscho, On 23.07.19 21:19, Johannes Schindelin wrote: > Hi Beat, > > On Mon, 22 Jul 2019, Beat Bolli wrote: > >> When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value >> of pcre1_jit_on. >> >> Print the value of pcre2_jit_on instead. >> >> Signed-off-by: Beat Bolli <dev+git@drbeat.li> >> --- >> grep.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/grep.c b/grep.c >> index f7c3a5803e..cd952ef5d3 100644 >> --- a/grep.c >> +++ b/grep.c >> @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt >> pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack); >> } else if (p->pcre2_jit_on != 0) { >> BUG("The pcre2_jit_on variable should be 0 or 1, not %d", >> - p->pcre1_jit_on); >> + p->pcre2_jit_on); > > Seems obviously good. > > Maybe while you're in the vicinity, you can add that information to the > `--debug` output? Do you mean something like this? diff --git a/grep.c b/grep.c index cd952ef5d3..8a57ba998f 100644 --- a/grep.c +++ b/grep.c @@ -406,6 +406,8 @@ static void compile_pcre1_regexp(struct grep_pat *p, const struct grep_opt *opt) #ifdef GIT_PCRE1_USE_JIT pcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on); + if (opt->debug) + fprintf(stderr, "pcre1_jit_on=%d\n", p->pcre1_jit_on); if (p->pcre1_jit_on == 1) { p->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024); if (!p->pcre1_jit_stack) @@ -522,6 +524,8 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt } pcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on); + if (opt->debug) + fprintf(stderr, "pcre2_jit_on=%d\n", p->pcre2_jit_on); if (p->pcre2_jit_on == 1) { jitret = pcre2_jit_compile(p->pcre2_pattern, PCRE2_JIT_COMPLETE); if (jitret) If so, I'll wait a bit until it's clear whether Ævar's series [1] or my patch is going to be applied. If this is missing your intentions, I need more guidance ;-) [1] https://public-inbox.org/git/20190724151415.3698-1-avarab@gmail.com/ > > Thanks, > Dscho > >> } >> } >> Cheers, Beat
Hi Beat, On Wed, 24 Jul 2019, Beat Bolli wrote: > On 23.07.19 21:19, Johannes Schindelin wrote: > > > > On Mon, 22 Jul 2019, Beat Bolli wrote: > > > >> When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value > >> of pcre1_jit_on. > >> > >> Print the value of pcre2_jit_on instead. > >> > >> Signed-off-by: Beat Bolli <dev+git@drbeat.li> > >> --- > >> grep.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/grep.c b/grep.c > >> index f7c3a5803e..cd952ef5d3 100644 > >> --- a/grep.c > >> +++ b/grep.c > >> @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt > >> pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack); > >> } else if (p->pcre2_jit_on != 0) { > >> BUG("The pcre2_jit_on variable should be 0 or 1, not %d", > >> - p->pcre1_jit_on); > >> + p->pcre2_jit_on); > > > > Seems obviously good. > > > > Maybe while you're in the vicinity, you can add that information to the > > `--debug` output? > > Do you mean something like this? Yes! > diff --git a/grep.c b/grep.c > index cd952ef5d3..8a57ba998f 100644 > --- a/grep.c > +++ b/grep.c > @@ -406,6 +406,8 @@ static void compile_pcre1_regexp(struct grep_pat *p, > const struct grep_opt *opt) > > #ifdef GIT_PCRE1_USE_JIT > pcre_config(PCRE_CONFIG_JIT, &p->pcre1_jit_on); > + if (opt->debug) > + fprintf(stderr, "pcre1_jit_on=%d\n", p->pcre1_jit_on); > if (p->pcre1_jit_on == 1) { > p->pcre1_jit_stack = pcre_jit_stack_alloc(1, 1024 * 1024); > if (!p->pcre1_jit_stack) > @@ -522,6 +524,8 @@ static void compile_pcre2_pattern(struct grep_pat > *p, const struct grep_opt *opt > } > > pcre2_config(PCRE2_CONFIG_JIT, &p->pcre2_jit_on); > + if (opt->debug) > + fprintf(stderr, "pcre2_jit_on=%d\n", p->pcre2_jit_on); > if (p->pcre2_jit_on == 1) { > jitret = pcre2_jit_compile(p->pcre2_pattern, > PCRE2_JIT_COMPLETE); > if (jitret) > > If so, I'll wait a bit until it's clear whether Ævar's series [1] or my > patch is going to be applied. Sounds fair. However, my reading of those patches is that they still keep support for JIT'ed and non-JIT'ed PCRE2 invocations (preferring the former if available, without any UI to change Git's behavior in that respect), so I would be surprised if this patch wasn't applicable after Ævar's patches. Thanks, Dscho > > If this is missing your intentions, I need more guidance ;-) > > [1] https://public-inbox.org/git/20190724151415.3698-1-avarab@gmail.com/ > > > > > > Thanks, > > Dscho > > > >> } > >> } > >> > > Cheers, > Beat >
On Thu, Jul 25, 2019 at 5:50 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > On Wed, 24 Jul 2019, Beat Bolli wrote: > > Do you mean something like this? > > Yes! Ideally, though, you want to print those values closing to the match function (pcre_exec for PCRE1 or pcre2_[jit_]match for PCRE2 as they might change after they are extracted from the PCRE configuration with pcre[2]-config() > > If so, I'll wait a bit until it's clear whether Ævar's series [1] or my > > patch is going to be applied. > > Sounds fair FWIW they are both applied (yours all the way to next), but Ævar's changes now in pu (but likely to be rerolled soon) will most likely remove your code (as explained on Junio's emails) > However, my reading of those patches is that they still keep support for > JIT'ed and non-JIT'ed PCRE2 invocations (preferring the former if > available, without any UI to change Git's behavior in that respect), so > I would be surprised if this patch wasn't applicable after Ævar's > patches. the PCRE1 changes are significant enough that would break the current check and might need a more convoluted check to figure out if JIT was actually used by pcre_exec. more details in the configuration[1] Carlo [1] https://www.pcre.org/original/doc/html/pcrejit.html
diff --git a/grep.c b/grep.c index f7c3a5803e..cd952ef5d3 100644 --- a/grep.c +++ b/grep.c @@ -559,7 +559,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack); } else if (p->pcre2_jit_on != 0) { BUG("The pcre2_jit_on variable should be 0 or 1, not %d", - p->pcre1_jit_on); + p->pcre2_jit_on); } }
When pcre2_jit_on is neither 1 nor 0, the BUG() call printed the value of pcre1_jit_on. Print the value of pcre2_jit_on instead. Signed-off-by: Beat Bolli <dev+git@drbeat.li> --- grep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)