Message ID | 55A66676.7070500@panix.com |
---|---|
State | New |
Headers | show |
Ping? 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. > > 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. > > - 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 > + <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')." > > #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); >
patch looks fine to me, but whether it makes it in 2.22 is up to Carlos -mike
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). Maybe, but only because I'd like someone to review it. I *may* get to reviewing it and push the patch myself, but I can't promise that, otherwise we'll get to it in 2.23. I apologize if that seems lame. We need more reviewers. c.
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.
On 07/24/2015 11:59 PM, Carlos O'Donell wrote: > On 07/15/2015 09:56 AM, Zack Weinberg wrote: >> +* 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. I'd like to keep the bug number in there, how's this? * The header <regexp.h> is deprecated, and will be removed in a future release. Use of this header will trigger a deprecation warning. Application developers should update their code to use <regex.h> instead. This header was formerly part of SUSv2, but was deprecated in 1997 and removed from the standard in 2001. Also, the glibc implementation leaks memory. See BZ#18681 for more details. >> -/* 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. Yes, POSIX issue 6. ("The Open Group Base Specifications Issue 6, IEEE Std 1003.1, 2004 Edition", which is the version more commonly known as POSIX.1-2001.) I tried to do some more research; unfortunately, the *previous* version of SUS (corresponding to POSIX.1-1996) identifies itself as "The Single Unix Specification, Version 2". I have not been able to pin down exactly what "issue 4, version 2" was, or find an "issue 5" anywhere; regexp.h is completely gone from "issue 6" and there is no overall change log; "version 2" already says NAME regexp.h - regular-expression declarations (LEGACY) ... APPLICATION USAGE This header is kept for historical reasons. New applications should use the regcomp(), regexec(), regerror() and regfree() functions, and the <regex.h> header ... and I would have to give them money to get my hands on "version 1." Given all that, how about this wording? +/* The contents of this header file were standardized in the + Single Unix Specification, Version 2 (1997) but marked as + LEGACY; new applications were already being encouraged to + use <regex.h> instead. POSIX.1-2001 removed this header. */ >> + 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 OK. >> + <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')" Actually, how about just #warning "<regexp.h> will be removed in the next release of the GNU C Library." #warning "Please update your code to use regex.h instead (no trailing 'p')." > Can you do a v3 with those changes for a final check? Coming up as soon as I complete some smoke tests. zw
On Sat, 25 Jul 2015, Zack Weinberg wrote: > itself as "The Single Unix Specification, Version 2". I have not been > able to pin down exactly what "issue 4, version 2" was, or find an See http://archive.opengroup.org/publications/archive/CDROM/ (specifically C434, C435, C436, C438; there are also corrigenda U038 U045 U052 U053 available separately somewhere). > "issue 5" anywhere; regexp.h is completely gone from "issue 6" and there > is no overall change log; "version 2" already says Issue 5 is the Single Unix Specification, Version 2.
On Mon, Jul 27, 2015 at 11:07 AM, Joseph Myers <joseph@codesourcery.com> wrote: > On Sat, 25 Jul 2015, Zack Weinberg wrote: > >> itself as "The Single Unix Specification, Version 2". I have not been >> able to pin down exactly what "issue 4, version 2" was, or find an > > See http://archive.opengroup.org/publications/archive/CDROM/ (specifically > C434, C435, C436, C438; there are also corrigenda U038 U045 U052 U053 > available separately somewhere). Thanks. Looking at those documents, the release history seems to be System V Release 2 (1984) (regexp.h invented) Issue 3 (????) (regexp.h standardized) Issue 4, version 2 (1994) (regexp.h marked TO BE WITHDRAWN) Issue 5 = Version 2 (1997) (no change) Issue 6 = POSIX.1-2001 (regexp.h deleted) probably with a bunch of intermediate editions in there, and of course the POSIX release history only converges with the SUS release history at -2001. I hope you can see why I was confused. So for perfect historical accuracy the comment should read /* The contents of this header file were standardized in the Single Unix Specification, Issue 3. In Issue 4 (1994) the header was marked as TO BE WITHDRAWN and new applications were encouraged to use <regex.h> instead. Issue 6 (aka POSIX.1-2001) removed the header entirely. */ ... and it would be nice to pin down a year for Issue 3 but I can't find one. zw
On Mon, 27 Jul 2015, Zack Weinberg wrote:
> ... and it would be nice to pin down a year for Issue 3 but I can't find one.
<http://pubs.opengroup.org/onlinepubs/009604499/frontmatter/refdocs.html>
says "1988, 1989, February 1992" (though all the individual component
documents are listed with "January 1989" as the first date, so I'm not
sure where 1988 comes in).
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. 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. - 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 + <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')." #include <features.h>