Discussion:
[PATCH 06/17] Regex: Use re_malloc / re_free consistently.
(too old to reply)
Paul Eggert
2017-12-19 23:57:37 UTC
Permalink
Raw Message
This patch changes several calls to malloc/free into re_malloc/re_free,
bringing consistency to the code.
Thanks, that patch makes sense, but it misses three opportunities to
bring consistency. regcomp.c has one call each to malloc and free, which
should be consistent too. Also, regexec.c has a call to realloc that
should be be changed to re_realloc. A minor formatting issue: one
newly-introduced re_malloc call doesn't need to appear on the next line.

(Possibly we should be adding consistency in the opposite way, by
removing the macros re_free, re_malloc, and re_realloc, and simply using
the underlying C functions. These macros are tricky since they are
function-like but (aside from re_free) cannot be implemented as
functions, and they don't buy much. But that'd be a bigger change.)

I installed the attached patch into Gnulib; it contains the originally
proposed patch 06/17 along with the abovementioned fixups. Something
like this should be easily installable into glibc.
a***@skeeve.com
2017-12-20 18:48:25 UTC
Permalink
Raw Message
Thanks, I have merged this in to gawk's version.

Paul --- I think that you have permission to push the patches you approve
to glibc. Please do so.

Thanks,

Arnold
Post by Paul Eggert
This patch changes several calls to malloc/free into re_malloc/re_free,
bringing consistency to the code.
Thanks, that patch makes sense, but it misses three opportunities to
bring consistency. regcomp.c has one call each to malloc and free, which
should be consistent too. Also, regexec.c has a call to realloc that
should be be changed to re_realloc. A minor formatting issue: one
newly-introduced re_malloc call doesn't need to appear on the next line.
(Possibly we should be adding consistency in the opposite way, by
removing the macros re_free, re_malloc, and re_realloc, and simply using
the underlying C functions. These macros are tricky since they are
function-like but (aside from re_free) cannot be implemented as
functions, and they don't buy much. But that'd be a bigger change.)
I installed the attached patch into Gnulib; it contains the originally
proposed patch 06/17 along with the abovementioned fixups. Something
like this should be easily installable into glibc.
Paul Eggert
2017-12-22 16:09:40 UTC
Permalink
Raw Message
Post by a***@skeeve.com
Paul --- I think that you have permission to push the patches you approve
to glibc. Please do so.
OK, I just now did that for patch 04/17, using a variant of Eric Blake's
original commit message for Gnulib (see attached). Carlos already installed
patch 01/17. As discussed, 02, 03, 05, 06, and 07 need thought before they can
go in, and the other patches I haven't reviewed yet. I'd like to finish
reviewing them all before installing patches that need thought.

Loading...