[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [patches] Add support for smaller regex implementation



On Tue, 11 Nov 2008, Maxim Kuvyrkov wrote:

> The interface to the library is the same whichever implementation is used with
> one exception: symbol re_max_failures, which was removed in GLIBC 2.4, is
> added back in libiberty's regex implementation.

What is the basis for this "removed in GLIBC 2.4"?  As far as I can see, 
on platforms where glibc 2.3 or earlier existed, current GLIBC still 
exports the symbol, complete with a default version so you can link new 
programs referring to it (with a link-time deprecation warning) as opposed 
to having no default symbol version so existing binaries continue to work 
but new ones cannot be linked using the symbol.

I'd think that having the symbol present at the same versions, with the 
same deprecation warning, would be appropriate for the alternative 
implementation.  For platforms where 2.4 or later is the earliest symbol 
version, the variable indeed is not exported, so I suppose there should be 
a hidden internal copy used by this implementation.  That is:

# if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_3)
(same as in present POSIX code)
# else
int re_max_failures attribute_hidden = (value in old code);
# endif

> Considering future maintenance, the whole libiberty's implementation is
> located in two files: xregex.c and xregex.h.  Xregex.h was copied without any
> change to it; xregex.c required small change to match ABI of current EGLIBC.

Installing xregex.h seems doubtful; there have been lots of changes to 
glibc's regex.h since then (or glibc/libiberty differences), some of which 
might affect the API exposed with various feature test macros.  I think 
that even if you use xregex.h internally to build xregex.c, you should 
install regex.h but possibly condition out any parts that do not match the 
exported interface of the old implementation, if using that 
implementation.

> Tested on powerpc-unknown-linux-gnu.  OK for trunk?

I'd have expected that the glibc regex implementation was replaced to fix 
problems with it, and there would be testcases for those problems.  Were 
there really no regex test failures with the libiberty implementation?  If 
some tests cover areas where the libiberty implementation fails, those 
tests should be disabled when using it.

-- 
Joseph S. Myers
joseph@xxxxxxxxxxxxxxxx