This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Kill regexp.h
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Zack Weinberg <zackw at panix dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Fri, 24 Jul 2015 23:59:45 -0400
- Subject: Re: [PATCH] Kill regexp.h
- Authentication-results: sourceware.org; auth=none
- References: <20150712195501 dot D8E5C14B9A at panix1 dot panix dot com> <55A57D59 dot 4090104 at redhat dot com> <CAKCAbMiGYx9dXsT3WF0ZQhum8757O-TWEhzzf_=mdRAO3rksXQ at mail dot gmail dot com> <55A65C27 dot 2020808 at redhat dot com> <55A66676 dot 7070500 at panix dot com>
On 07/15/2015 09:56 AM, Zack Weinberg wrote:
> On 07/15/2015 09:12 AM, Carlos O'Donell wrote:
>> On 07/14/2015 08:20 PM, Zack Weinberg wrote:
>>>> At a high level your patch looks OK, it makes sense to deprecate
>>>> these interfaces, but I think we should to do this in two stages.
>>>> Add warnings and then remove.
>>>
>>> Hm. If we do that then I would feel obliged to fix the bugs in the
>>> header in phase one. I'm not sure that's worth doing...
>>
>> Worth is certainly in the eye of the beholder. How would you feel if
>> you were a user of this interface?
>
> So thinking about this a bit more, would you take this patch for 2.22?
> All it does is add #warning directives and correct the RETURN .vs. ERROR
> thing (leaving the memory-allocation issue reported in Debian).
>
> ---
> * regexp.h: Add unconditional #warning stating that this header
> will be removed soon. Revise banner comment to match.
> (compile): Consistently use ERROR instead of RETURN to report
> errors (partial fix for bz#18681).
> * regexp.c: Don't include regexp.h.
> ---
> NEWS | 6 ++++++
> misc/regexp.c | 8 ++++----
> misc/regexp.h | 30 +++++++++++++++++-------------
> 3 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index f91edc7..d9cf4a7 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -71,6 +71,12 @@ Version 2.22
> compliance. The new implementation fixes the following long-standing
> issues: BZ#6544, BZ#11216, BZ#12836, BZ#13151, BZ#13152, and
> BZ#14292. The
> old implementation is still present for use be by existing binaries.
> +
> +* The header <regexp.h> is deprecated, and will be removed in a future
> + release. (It was removed from POSIX long ago, and it has bugs we cannot
> + easily fix. See BZ#18681 for more details.) We suspect that no one has
> + used this header in many years, but if you have code that does use it,
> + you will need to update it to use <regex.h> instead.
Suggest:
* The header <regexp.h> is deprecated, and will be removed in a future
release. As of 1997, SUSv2 marked this interface as legacy and recommended
applications migrate. Application developers are expected to update to
<regex.h> instead. Use of the <regexp.h> header will trigger a
deprecation warning.
>
> Version 2.21
>
> diff --git a/misc/regexp.c b/misc/regexp.c
> index 3b83203..d389804 100644
> --- a/misc/regexp.c
> +++ b/misc/regexp.c
> @@ -17,8 +17,10 @@
> License along with the GNU C Library; if not, see
> <http://www.gnu.org/licenses/>. */
>
> -#define __DO_NOT_DEFINE_COMPILE
> -#include <regexp.h>
> +/* We don't include regexp.h here because of the macros it requires, and
> + because it now contains an unconditional #warning. */
> +
> +#include <regex.h>
>
> /* Define the variables used for the interface. */
> char *loc1;
> @@ -32,7 +34,6 @@ char *locs;
> found in the buffer starting at EXPBUF. `loc1' will return the
> first character matched and `loc2' points to the next unmatched
> character. */
> -extern int __step (const char *string, const char *expbuf);
> int
> __step (const char *string, const char *expbuf)
> {
> @@ -55,7 +56,6 @@ weak_alias (__step, step)
> /* Match the beginning of STRING with the compiled regular expression
> in EXPBUF. If the match is successful `loc2' will contain the
> position of the first unmatched character. */
> -extern int __advance (const char *string, const char *expbuf);
> int
> __advance (const char *string, const char *expbuf)
> {
> diff --git a/misc/regexp.h b/misc/regexp.h
> index 3fc0bc5..f8eefe4 100644
> --- a/misc/regexp.h
> +++ b/misc/regexp.h
> @@ -19,14 +19,18 @@
> #ifndef _REGEXP_H
> #define _REGEXP_H 1
>
> -/* The contents of this header file was first standardized in X/Open
> +/* The contents of this header file were first standardized in X/Open
> System Interface and Headers Issue 2, originally coming from SysV.
> - In issue 4, version 2, it is marked as TO BE WITDRAWN, and it has
> - been withdrawn in SUSv3.
> + In issue 4, version 2, it was marked as TO BE WITHDRAWN, and it was
> + duly withdrawn in issue 6.
Issue 6 of what? I assume you mean POSIX issue 6? Please clarify.
>
> - This code shouldn't be used in any newly written code. It is
> - included only for compatibility reasons. Use the POSIX definition
> - in <regex.h> for portable applications and a reasonable interface. */
> + This header is provided only for backward compatibility, and it will
> + be removed in the next release of GNU libc. New code should use
s/GNU libc/GNU C Library/g
> + <regex.h> instead. */
> +
> +#warning "regexp.h is obsolete and buggy."
> +#warning "It will be removed in the next release of GNU libc."
> +#warning "Please update your code to use regex.h instead (no trailing
> 'p')."
Suggest:
#warning "Use of the <regexp.h> header is deprecated."
#warning "It will be removed in the next release of the GNU C Library."
#warning "Please use <regex.h> instead (no trailing 'p')"
>
> #include <features.h>
> #include <alloca.h>
> @@ -182,19 +186,19 @@ compile (char *__restrict instring, char
> *__restrict expbuf,
> case REG_ERPAREN:
> default:
> /* There is no matching error code. */
> - RETURN (36);
> + ERROR (36);
> case REG_ESUBREG:
> - RETURN (25);
> + ERROR (25);
> case REG_EBRACK:
> - RETURN (49);
> + ERROR (49);
> case REG_EPAREN:
> - RETURN (42);
> + ERROR (42);
> case REG_EBRACE:
> - RETURN (44);
> + ERROR (44);
> case REG_BADBR:
> - RETURN (46);
> + ERROR (46);
> case REG_ERANGE:
> - RETURN (11);
> + ERROR (11);
> case REG_ESPACE:
> case REG_ESIZE:
> ERROR (50);
OK.
Can you do a v3 with those changes for a final check?
patchwork workflow is easier that way.
Cheers,
Carlos.