Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> writes:
As we already have such an ifdef block here...
> +#ifdef USE_LIBPCRE2
> +static pcre2_general_context *pcre2_global_context;
> +
> +#ifdef USE_NED_ALLOCATOR
> +static void *pcre2_malloc(PCRE2_SIZE size, MAYBE_UNUSED void *memory_data)
> +{
> + return xmalloc(size); /* will use nedalloc underneath */
> +}
> +
> +static void pcre2_free(void *pointer, MAYBE_UNUSED void *memory_data)
> +{
> + return free(pointer);
> +}
> +#endif
> +#endif
... perhaps an ugly ifdef block like this one ...
> +static struct grep_pat *create_grep_pat(struct grep_opt *opt,
> + const char *pat, size_t patlen,
> const char *origin, int no,
> enum grep_pat_token t,
> enum grep_header_field field)
> {
> struct grep_pat *p = xcalloc(1, sizeof(*p));
> +
> +#if defined(USE_LIBPCRE2) && defined(USE_NED_ALLOCATOR)
> + /* BUG: this is technically not needed if we will do UTF matching
> + * but UTF locale detection is currently broken */
> + /* SMELL: opt shouldn't be needed at this level but it is used
> + * here to keep the way we were detecting the need for
> + * the chartable consistent and until it can be refactored
> + * properly. the NULL check is needed as a workaround
> + * for segfaulting in t7810.102 and should also go */
> + /* TODO: has_non_ascii doesn't support NUL in pattern */
> + if (!pcre2_global_context && opt && opt->ignore_case &&
> + has_non_ascii(pat))
> + pcre2_global_context = pcre2_general_context_create(
> + pcre2_malloc, pcre2_free, NULL);
> +#endif
> +
... can be abstracted away by *not* having the #if/#endif here and
instead have a line that looks like this:
setup_pcre2_as_needed(opt, pat);
which implementation can be prepared near the top, close to where
pcre2_malloc/free are defined (or not) above, i.e.
#if defined(USE_LIBPCRE2) && defined(USE_NED_ALLOCATOR)
static void setup_pcre2_as_needed(struct grep_opt *opt, const char *pat)
{
... the above one ...
}
#else
#define setup_pcre2_as_needed(ignore1, ignore2) /* nothing */
#endif
The conditional code in grep_destroy() can be eliminated in a
similar fashion, i.e.
void grep_destroy(void)
{
cleanup_pcre2_as_needed();
}
with
#ifdef USE_LIBPCRE2
static void cleanup_pcre2_as_needed(void)
{
pcre2_general_context_free(pcre2_global_context);
}
#else
#define cleanup_pcre2_as_needed() /* nothing */
#endif
near the top (the beneral idea is to "call" a helper function whose
name describes what the call is for in a more general terms, and let
only the implementation details be hidden inside ifdef blocks).
Also,
/*
* our multi-line comments are supposed to be formatted like
* this, with opening and closing slash-asterisk and asterisk-slash
* on their own lines.
*/
