From: Carlo Arenas <carenas@gmail.com> To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com> Cc: git@vger.kernel.org Subject: Re: [PATCH 1/3] grep: make pcre1_tables version agnostic Date: Sat, 27 Jul 2019 19:50:07 -0700 [thread overview] Message-ID: <CAPUEspg255TAE-1ozc8CuC=k9oeP_WLD4TrFur5p=XY3s8PX3Q@mail.gmail.com> (raw) In-Reply-To: <875znn6nvt.fsf@evledraar.gmail.com> On Sat, Jul 27, 2019 at 4:47 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > > On Sat, Jul 27 2019, Carlo Marcelo Arenas Belón wrote: > > > 6d4b5747f0 ("grep: change internal *pcre* variable & function names > > to be *pcre1*", 2017-05-25), renamed most variables to be PCRE1 > > specific to give space to similarly named variables for PCRE2, but > > in this case the change wasn't needed as the types were compatible > > enough (const unsigned char* vs const uint8_t*) to be shared. reading this again, had to admit there is a fair amount of guessing on intent, but reading commits and the email discussion couldn't come up with a better explanation. is the root cause for the bug different?, could it be that the pcre2 API was misunderstood? and you expected this pointer will be free together with the context? (as it is done when a cloned context with tables is used?) > Both the v1 and v2 functions return const unsigned char *. I don't know > where I got the uint8_t from. This makes more sense. the type in PCRE2 is uint8_t, the documentation has a bug[1] almost every platform git cares for would have unsigned char = uint8_t though. > The point of 6d4b5747f0 was not to only split out those variables we > couldn't get away with re-using. Then I would have later re-used > e.g. pcre1_jit_on & pcre2_jit_on as just pcre_jit_on. We could also do > that now. IMHO pcre_jit_on makes more sense as a bit, with local variables being used for the pcre*_config() call with the right type.(uint32_t != int) > I think doing that & this part of the your changes makes things less > readable. The two code branches we compile with ifdefs are mutually > exclusive, so having the variables be unique helps with eyeballing / > reasoning when changing the code. I thought that too until the typo[2] in the pcre?_jit_on variable (now in next) kind of proved us wrong. Maybe the names are too similar? either way, would you rather drop this patch and make a replica variable? should I rebase to next with Reviewed-By tags so that all other changes in flight that would conflict could be corrected? Carlo [1] https://bugs.exim.org/show_bug.cgi?id=2420 [2] https://public-inbox.org/git/20190722181923.21572-1-dev+git@drbeat.li/
next prev parent reply other threads:[~2019-07-28 2:50 UTC|newest] Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-27 20:27 [PATCH 0/3] grep: memory leak in PCRE2 Carlo Marcelo Arenas Belón 2019-07-27 20:27 ` [PATCH 1/3] grep: make pcre1_tables version agnostic Carlo Marcelo Arenas Belón 2019-07-27 23:47 ` Ævar Arnfjörð Bjarmason 2019-07-28 2:50 ` Carlo Arenas [this message] 2019-07-27 20:27 ` [PATCH 2/3] grep: use pcre_tables also for PCRE2 Carlo Marcelo Arenas Belón 2019-07-27 20:27 ` [PATCH 3/3] grep: plug leak of pcre chartables in PCRE2 Carlo Marcelo Arenas Belón 2019-07-27 23:48 ` Ævar Arnfjörð Bjarmason 2019-07-28 1:41 ` Carlo Arenas 2019-07-29 20:34 ` René Scharfe 2019-07-30 0:08 ` Carlo Arenas 2019-07-30 16:52 ` René Scharfe 2019-08-01 17:09 ` [PATCH v2] grep: avoid leak of " Carlo Marcelo Arenas Belón 2019-08-02 16:19 ` Junio C Hamano 2019-08-03 18:50 ` Carlo Arenas 2019-08-05 19:34 ` Junio C Hamano
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAPUEspg255TAE-1ozc8CuC=k9oeP_WLD4TrFur5p=XY3s8PX3Q@mail.gmail.com' \ --to=carenas@gmail.com \ --cc=avarab@gmail.com \ --cc=git@vger.kernel.org \ --subject='Re: [PATCH 1/3] grep: make pcre1_tables version agnostic' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).