Re: [PATCH] Kill regexp.h

[Zack Weinberg <zackw at panix dot com>]

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

Attachment: signature.asc
Description: OpenPGP digital signature