Kill regexp.h

  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(-)

 #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);
  

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).