GNU bug report logs - #17157
[PATCH 1/5] Partially revert "dfa: improve port to freestanding DJGPP"

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17157 in the body.
You can then email your comments to 17157 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paolo Bonzini <bonzini <at> gnu.org>
To: bug-grep <at> gnu.org
Cc: meyering <at> fb.com
Subject: [PATCH 1/5] Partially revert "dfa: improve port to freestanding DJGPP"
Date: Tue,  1 Apr 2014 11:18:42 +0200

This partially reverts the following commits:
- df6da5d40a47abbc6e3451cb9fab7a8c9ede12cc.
- c40e7f7158109985b901ae67e5faa06160547471.
- 0995fc11214ebbb1c99a4b6d675fe615b0ed8539.

As far as grep is concerned, the #define *does* belong in gnulib.  gawk
can apply the fix in a common header file like grep's src/system.h.
---
 src/dfa.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/src/dfa.c b/src/dfa.c
index b22fe97..a7f0056 100644
--- a/src/dfa.c
+++ b/src/dfa.c
@@ -34,11 +34,7 @@
 #include <locale.h>
 #include <stdbool.h>
 
-/* Gawk doesn't use Gnulib, so don't assume that setlocale and
-   static_assert are present.  */
-#ifndef LC_ALL
-# define setlocale(category, locale) NULL
-#endif
+/* Gawk doesn't use Gnulib, so don't assume static_assert is present.  */
 #ifndef static_assert
 # define static_assert(cond, diagnostic) \
     extern int (*foo (void)) [!!sizeof (struct { int foo: (cond) ? 8 : -1; })]
-- 
1.9.0

Message #8 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: bonzini <at> gnu.org, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 03:33:32 -0600

Hi.

Please don't revert this change. dfa.c is *shared* code, and there's no
reason for gawk to put stuff needed ONLY by dfa.c into its config.h
file. (Gawk doesn't have a system.h - most of that stuff is in awk.h
at the momment. The rest is in custom.h which is used to override defaults
from configure, and not for portability.)

I understand that grep doesn't need it, but dfa serves more than
one master. I appreciate the grep team's accommodating gawk's needs.

Thanks,

Arnold
------------------

Paolo Bonzini <bonzini <at> gnu.org> wrote:

> This partially reverts the following commits:
> - df6da5d40a47abbc6e3451cb9fab7a8c9ede12cc.
> - c40e7f7158109985b901ae67e5faa06160547471.
> - 0995fc11214ebbb1c99a4b6d675fe615b0ed8539.
>
> As far as grep is concerned, the #define *does* belong in gnulib.  gawk
> can apply the fix in a common header file like grep's src/system.h.
> ---
>  src/dfa.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/src/dfa.c b/src/dfa.c
> index b22fe97..a7f0056 100644
> --- a/src/dfa.c
> +++ b/src/dfa.c
> @@ -34,11 +34,7 @@
>  #include <locale.h>
>  #include <stdbool.h>
>  
> -/* Gawk doesn't use Gnulib, so don't assume that setlocale and
> -   static_assert are present.  */
> -#ifndef LC_ALL
> -# define setlocale(category, locale) NULL
> -#endif
> +/* Gawk doesn't use Gnulib, so don't assume static_assert is present.  */
>  #ifndef static_assert
>  # define static_assert(cond, diagnostic) \
>      extern int (*foo (void)) [!!sizeof (struct { int foo: (cond) ? 8 : -1; })]
> -- 
> 1.9.0
>
>
>
>
> From bug-grep-bounces+arnold=skeeve.com <at> gnu.org  Tue Apr  1 03:19:14 2014
> X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on
> 	frenzy.freefriends.org
> X-Spam-Level: 
> X-Spam-Status: No, score=-7.5 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI,
> 	RP_MATCHES_RCVD autolearn=ham version=3.3.1
> X-Envelope-From: bug-grep-bounces+arnold=skeeve.com <at> gnu.org
> Return-Path: <bug-grep-bounces+arnold=skeeve.com <at> gnu.org>
> X-Loop: help-debbugs <at> gnu.org
> Subject: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
> 	freestanding DJGPP"
> Resent-From: Paolo Bonzini <bonzini <at> gnu.org>
> Original-Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
> Resent-CC: bug-grep <at> gnu.org
> Resent-Date: Tue, 01 Apr 2014 09:19:03 +0000
> Resent-Message-ID: <handler.17157.B.139634394319944 <at> debbugs.gnu.org>
> Resent-Sender: help-debbugs <at> gnu.org
> X-GNU-PR-Message: report 17157
> X-GNU-PR-Package: grep
> X-GNU-PR-Keywords: patch
> To: 17157 <at> debbugs.gnu.org
> X-Debbugs-Original-To: bug-grep <at> gnu.org
> From: Paolo Bonzini <bonzini <at> gnu.org>
> Date: Tue,  1 Apr 2014 11:18:42 +0200
> X-Mailer: git-send-email 1.9.0
> X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address
> 	(bad octet value).
> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x
> X-BeenThere: debbugs-submit <at> debbugs.gnu.org
> X-Mailman-Version: 2.1.15
> Precedence: list
> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x
> X-Received-From: 140.186.70.43
> Cc: meyering <at> fb.com
> X-BeenThere: bug-grep <at> gnu.org
> List-Id: Bug reports for GNU grep <bug-grep.gnu.org>
> List-Unsubscribe: <https://lists.gnu.org/mailman/options/bug-grep>,
> 	<mailto:bug-grep-request <at> gnu.org?subject=unsubscribe>
> List-Archive: <http://lists.gnu.org/archive/html/bug-grep>
> List-Post: <mailto:bug-grep <at> gnu.org>
> List-Help: <mailto:bug-grep-request <at> gnu.org?subject=help>
> List-Subscribe: <https://lists.gnu.org/mailman/listinfo/bug-grep>,
> 	<mailto:bug-grep-request <at> gnu.org?subject=subscribe>
> Errors-To: bug-grep-bounces+arnold=skeeve.com <at> gnu.org
> Sender: bug-grep-bounces+arnold=skeeve.com <at> gnu.org
> Status: R
>
> This partially reverts the following commits:
> - df6da5d40a47abbc6e3451cb9fab7a8c9ede12cc.
> - c40e7f7158109985b901ae67e5faa06160547471.
> - 0995fc11214ebbb1c99a4b6d675fe615b0ed8539.
>
> As far as grep is concerned, the #define *does* belong in gnulib.  gawk
> can apply the fix in a common header file like grep's src/system.h.
> ---
>  src/dfa.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/src/dfa.c b/src/dfa.c
> index b22fe97..a7f0056 100644
> --- a/src/dfa.c
> +++ b/src/dfa.c
> @@ -34,11 +34,7 @@
>  #include <locale.h>
>  #include <stdbool.h>
>  
> -/* Gawk doesn't use Gnulib, so don't assume that setlocale and
> -   static_assert are present.  */
> -#ifndef LC_ALL
> -# define setlocale(category, locale) NULL
> -#endif
> +/* Gawk doesn't use Gnulib, so don't assume static_assert is present.  */
>  #ifndef static_assert
>  # define static_assert(cond, diagnostic) \
>      extern int (*foo (void)) [!!sizeof (struct { int foo: (cond) ? 8 : -1; })]
> -- 
> 1.9.0
>
>
>
>
>

Message #11 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paolo Bonzini <bonzini <at> gnu.org>
To: arnold <at> skeeve.com, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 11:47:55 +0200

Il 01/04/2014 11:33, arnold <at> skeeve.com ha scritto:
> Please don't revert this change. dfa.c is *shared* code, and there's no
> reason for gawk to put stuff needed ONLY by dfa.c into its config.h
> file. (Gawk doesn't have a system.h - most of that stuff is in awk.h
> at the momment. The rest is in custom.h which is used to override defaults
> from configure, and not for portability.)
>
> I understand that grep doesn't need it, but dfa serves more than
> one master. I appreciate the grep team's accommodating gawk's needs.

Then we should add a separate header for this purpose.

One thing is serving more than one master and having the occasional 
#ifndef GREP, another thing is doing things twice in grep (via gnulib 
and explicitly in dfa.c).

setlocale, static_assert etc. should be in a dfacompat.h file that is 
not shared between grep and gawk and could also include the #define for 
GREP and GAWK symbols.

Paolo

Message #14 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: bonzini <at> gnu.org, arnold <at> skeeve.com, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 04:54:17 -0600

Hi Paolo.

> One thing is serving more than one master and having the occasional 
> #ifndef GREP, another thing is doing things twice in grep (via gnulib 
> and explicitly in dfa.c).
>
> setlocale, static_assert etc. should be in a dfacompat.h file that is 
> not shared between grep and gawk and could also include the #define for 
> GREP and GAWK symbols.

I could live with that.  It's a good idea.

Thanks,

Arnold

Message #17 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: bonzini <at> gnu.org, arnold <at> skeeve.com, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 06:00:51 -0600

Hi.

> > One thing is serving more than one master and having the occasional 
> > #ifndef GREP, another thing is doing things twice in grep (via gnulib 
> > and explicitly in dfa.c).
> >
> > setlocale, static_assert etc. should be in a dfacompat.h file that is 
> > not shared between grep and gawk and could also include the #define for 
> > GREP and GAWK symbols.
>
> I could live with that.  It's a good idea.

I should point out that defining GAWK in dfacompat.h is not the right
thing - I define it globally since there are a few gawk-specific bits in
the regex code too.

Thanks,

Arnold

Message #20 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Paolo Bonzini <bonzini <at> gnu.org>, arnold <at> skeeve.com, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 07:43:00 -0700

Paolo Bonzini wrote:
> setlocale, static_assert etc. should be in a dfacompat.h file

We already have a file that serves this function, called mbsupport.h. 
We could rename it to dfaconfig.h (a better name than dfacompat.h, as 
it's basically a separate config.h for dfa.c).

While we're on the topic, does anybody build gawk or grep with 
-DMBS_SUPPORT=0?  I suspect not, which means we could simplify 
dfaconfig.h compared to mbsupport.h.  (And if somebody does build it 
that way, I suspect it's buggy....)

Message #23 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paolo Bonzini <bonzini <at> gnu.org>
To: Paul Eggert <eggert <at> cs.ucla.edu>, arnold <at> skeeve.com, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 17:08:50 +0200

Il 01/04/2014 16:43, Paul Eggert ha scritto:
> Paolo Bonzini wrote:
>> setlocale, static_assert etc. should be in a dfacompat.h file
>
> We already have a file that serves this function, called mbsupport.h. We
> could rename it to dfaconfig.h (a better name than dfacompat.h, as it's
> basically a separate config.h for dfa.c).

Good idea.

Paolo

Message #26 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: eggert <at> cs.ucla.edu, bonzini <at> gnu.org, arnold <at> skeeve.com,
 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Tue, 01 Apr 2014 12:36:08 -0600

Hi Paul & Paolo,

Paolo Bonzini <bonzini <at> gnu.org> wrote:

> Il 01/04/2014 16:43, Paul Eggert ha scritto:
> > Paolo Bonzini wrote:
> >> setlocale, static_assert etc. should be in a dfacompat.h file
> >
> > We already have a file that serves this function, called mbsupport.h. We
> > could rename it to dfaconfig.h (a better name than dfacompat.h, as it's
> > basically a separate config.h for dfa.c).
>
> Good idea.
>
> Paolo

Not really such a good idea. mbsupport.h is used in multiple places
in gawk to define if MBS support is availble. It needs to stay
that way. Since we're discussing keeping dfa simple in both its
environments, then it needs to be in a header that only dfa uses.

At the moment, the best candidate is xalloc.h, which differs somewhat
grep vs. gawk. 

But here too, in reality that file is mainly for allocation stuff
and dfa portability doesn't really belong there.

I respectfully suggest dfaconfig.h (or dfacustom.h) with

	#ifdef GAWK
	#ifndef HAVE_SETLOCALE
	#define selocale(x, y)	NULL
	#endif
	#define static_assert(cond) ...
	#endif

as the initial contents.

Much thanks,

Arnold

Message #29 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paolo Bonzini <bonzini <at> gnu.org>
To: arnold <at> skeeve.com, eggert <at> cs.ucla.edu, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Wed, 02 Apr 2014 09:10:42 +0200

Il 01/04/2014 20:36, arnold <at> skeeve.com ha scritto:
> Not really such a good idea. mbsupport.h is used in multiple places
> in gawk to define if MBS support is availble. It needs to stay
> that way. Since we're discussing keeping dfa simple in both its
> environments, then it needs to be in a header that only dfa uses.
>
> At the moment, the best candidate is xalloc.h, which differs somewhat
> grep vs. gawk.
>
> But here too, in reality that file is mainly for allocation stuff
> and dfa portability doesn't really belong there.
>
> I respectfully suggest dfaconfig.h (or dfacustom.h) with
>
> 	#ifdef GAWK
> 	#ifndef HAVE_SETLOCALE
> 	#define selocale(x, y)	NULL
> 	#endif
> 	#define static_assert(cond) ...
> 	#endif
>
> as the initial contents.

Couldn't gawk's dfaconfig.h include gawk's mbsupport.h?

Paolo

Message #32 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: eggert <at> cs.ucla.edu, bonzini <at> gnu.org, arnold <at> skeeve.com,
 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Wed, 02 Apr 2014 01:40:15 -0600

Paul Eggert <eggert <at> cs.ucla.edu> wrote:

> While we're on the topic, does anybody build gawk or grep with 
> -DMBS_SUPPORT=0?  I suspect not, which means we could simplify 
> dfaconfig.h compared to mbsupport.h.  (And if somebody does build it 
> that way, I suspect it's buggy....)

MBS_SUPPORT is defined by the checks in mbsupport.h - noone should
be setting it on the command line.

I build gawk that way once in a blue moon just to test.

It may be that VMS and OS/2 don't have the MBS support, I'm not
sure.

Please see my other mail about why mbsupport.h cannot be renamed
dfaconfig.h and that a separate file is needed.

Thanks,

Arnold

Message #35 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: arnold <at> skeeve.com
To: eggert <at> cs.ucla.edu, bonzini <at> gnu.org, arnold <at> skeeve.com,
 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Wed, 02 Apr 2014 01:59:02 -0600

Paolo Bonzini <bonzini <at> gnu.org> wrote:

> Il 01/04/2014 20:36, arnold <at> skeeve.com ha scritto:
> > Not really such a good idea. mbsupport.h is used in multiple places
> > in gawk to define if MBS support is availble. It needs to stay
> > that way. Since we're discussing keeping dfa simple in both its
> > environments, then it needs to be in a header that only dfa uses.
> >
> > At the moment, the best candidate is xalloc.h, which differs somewhat
> > grep vs. gawk.
> >
> > But here too, in reality that file is mainly for allocation stuff
> > and dfa portability doesn't really belong there.
> >
> > I respectfully suggest dfaconfig.h (or dfacustom.h) with
> >
> > 	#ifdef GAWK
> > 	#ifndef HAVE_SETLOCALE
> > 	#define selocale(x, y)	NULL
> > 	#endif
> > 	#define static_assert(cond) ...
> > 	#endif
> >
> > as the initial contents.
>
> Couldn't gawk's dfaconfig.h include gawk's mbsupport.h?

Yes, but why? dfa.c needs mbsuport.h no matter what.

Using Paul's separation of concerns:

* mbsupport.h - provide definition for MBSUPPORT. Opaque to dfa.c.
* xalloc.h - provide declarations (and maybe definitions) for **alloc,
             opaque to dfa.c
* dfaconfg.h - provide other definitions needed by dfa.c which may not be
               in standard headers for when gnulib isn't being used

Thanks,

Arnold

Message #38 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paolo Bonzini <bonzini <at> gnu.org>
To: arnold <at> skeeve.com, eggert <at> cs.ucla.edu, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Wed, 02 Apr 2014 13:19:30 +0200

Il 02/04/2014 09:59, arnold <at> skeeve.com ha scritto:
> Using Paul's separation of concerns:
>
> * mbsupport.h - provide definition for MBSUPPORT. Opaque to dfa.c.
> * xalloc.h - provide declarations (and maybe definitions) for **alloc,
>              opaque to dfa.c
> * dfaconfg.h - provide other definitions needed by dfa.c which may not be
>                in standard headers for when gnulib isn't being used

You're right, I was missing that grep uses mbsupport.h elsewhere than in 
dfa.c.

Paolo

Message #41 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: arnold <at> skeeve.com, bonzini <at> gnu.org, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Wed, 02 Apr 2014 13:58:37 -0700

On 04/02/2014 12:40 AM, arnold <at> skeeve.com wrote:

> It may be that VMS and OS/2 don't have the MBS support

VMS and OS/2 both have MBS support, as far as I can see.  Certainly 
OpenVMS does.

Gawk's ChangeLogs suggest that mbsupport.h is needed only to support a 
port of Gawk to VAX C circa 2004, one where Pat Rankin ran into a 
problem with VAX C.  This predates the gawk git first checkin, and 
predates bug-gawk, so I don't know what the problem was -- all I know is 
the bit of info that's in the Gawk changelogs.  However, whatever the 
problem was, it should be possible to work around it without affecting 
dfa.c, and dfa.c should be able to be consistent about simply using 
MB_CUR_MAX to test whether we're in a unibyte locale.

I'll look into writing patches for that, one for grep (which affects 
dfa.c), one for gawk (which will use the same patches to dfa.c).

Message #44 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: arnold <at> skeeve.com, bonzini <at> gnu.org, 17157 <at> debbugs.gnu.org
Cc: meyering <at> fb.com
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Thu, 03 Apr 2014 19:21:56 -0700

[Message part 1 (text/plain, inline)]

Paul Eggert wrote:
> I'll look into writing patches for that, one for grep (which affects
> dfa.c), one for gawk (which will use the same patches to dfa.c).

OK, I got it to work, and it simplifies grep (not surprising) and gawk 
as well (a bit surprising, but there it is).  I'm attaching the patches, 
one for each program.  These patches include Paolo's original suggestion 
at the start of bug#17157, plus several other simplifications to dfa.c.

[0001-grep-simplify-dfa.c-by-having-it-not-include-mbsuppo.patch (text/plain, attachment)]

[0001-awk-simplify-dfa.c-by-having-it-not-include-mbsuppor.patch (text/plain, attachment)]

Message #47 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Jim Meyering <meyering <at> fb.com>, Paolo Bonzini <bonzini <at> gnu.org>,
 Aharon Robbins <arnold <at> skeeve.com>, 17157 <at> debbugs.gnu.org
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Fri, 4 Apr 2014 08:04:28 -0700

On Thu, Apr 3, 2014 at 7:21 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
> Paul Eggert wrote:
>>
>> I'll look into writing patches for that, one for grep (which affects
>> dfa.c), one for gawk (which will use the same patches to dfa.c).
>
> OK, I got it to work, and it simplifies grep (not surprising) and gawk as
> well (a bit surprising, but there it is).  I'm attaching the patches, one
> for each program.  These patches include Paolo's original suggestion at the
> start of bug#17157, plus several other simplifications to dfa.c.

Thanks!  The grep changes all look fine.

Message #52 received at 17157-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Jim Meyering <jim <at> meyering.net>
Cc: Paolo Bonzini <bonzini <at> gnu.org>, Aharon Robbins <arnold <at> skeeve.com>,
 17157-done <at> debbugs.gnu.org
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Sat, 05 Apr 2014 16:36:20 -0700

Jim Meyering wrote:
> Thanks!  The grep changes all look fine.

OK, thanks, I installed them and am marking this bug as done.  As I 
understand it, Aharon is in the middle of a gawk release and so now's 
not a good time to install the gawk changes, but we can address any 
integration issues if they later come up.

Message #55 received at 17157-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Meyering <jim <at> meyering.net>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Paolo Bonzini <bonzini <at> gnu.org>, Aharon Robbins <arnold <at> skeeve.com>,
 17157-done <at> debbugs.gnu.org
Subject: Re: bug#17157: [PATCH 1/5] Partially revert "dfa: improve port to
 freestanding DJGPP"
Date: Sun, 6 Apr 2014 10:01:15 -0700

[Message part 1 (text/plain, inline)]

On Sat, Apr 5, 2014 at 4:36 PM, Paul Eggert <eggert <at> cs.ucla.edu> wrote:
> Jim Meyering wrote:
>>
>> Thanks!  The grep changes all look fine.
>
>
> OK, thanks, I installed them and am marking this bug as done.  As I
> understand it, Aharon is in the middle of a gawk release and so now's not a
> good time to install the gawk changes, but we can address any integration
> issues if they later come up.

Hi Paul,

I see that dfa.c now fails to compile on systems without static_assert
support, e.g., OS X 10.8.5 (even with gcc-4.9.0 20140324):

dfa.c:918:16: error: expected declaration specifiers or '...' before '(' token
 static_assert ((sizeof lonesome_lower / sizeof *lonesome_lower + 2

I've pushed the attached to make grep buildable again.

[k.txt (text/plain, attachment)]

Message #58 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: 17072-done <at> debbugs.gnu.org
Cc: 17157 <at> debbugs.gnu.org, bug-gawk <at> gnu.org,
 "Nelson H. F. Beebe" <beebe <at> math.utah.edu>
Subject: Re: bug#17072: dfa change apparently needed on Irix
Date: Sun, 06 Apr 2014 11:00:58 -0700

[Message part 1 (text/plain, inline)]

Thanks to Nelson H.F. Beebe's machines I've reproduced Bug#17072 on IRIX 
and have verified that the attached gawk patch fixes it.  This patch is 
almost identical to the gawk patch I submitted in 
<http://bugs.gnu.org/17157#44>.  It improves on the earlier patch only 
by changing gawk's dfa.c to look more like grep's dfa.c in one more way: 
include "dfa.h" before all other include files (except config.h).

I'll CC: this to Bug#17157 so that the patch is visible there too.  No 
further change should be needed to grep for this.

[0001-awk-simplify-dfa.c-by-having-it-not-include-mbsuppor.patch (text/plain, attachment)]

Message #61 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: arnold <at> skeeve.com, 17072 <at> debbugs.gnu.org
Cc: 17157 <at> debbugs.gnu.org
Subject: Re: bug#17072: closed (Re: bug#17072: dfa change apparently needed
 on Irix)
Date: Sun, 06 Apr 2014 14:26:03 -0700

[Message part 1 (text/plain, inline)]

arnold <at> skeeve.com wrote:
> 2013-01-31         Arnold D. Robbins<arnold <at> skeeve.com>
>
> 	* dfa.c: Include "dfa.h" which includes regex.h after limits.h
> 	so that RE_DUP_MAX gets the correct value.

Sorry, I missed that one.  We ran into that problem in Gnulib in March 
2012 and I should have included the fix in the proposed Gawk patch. 
Revised Gawk patch attached.  This affects only regex.h (to make it like 
gnulib regex.h with respect to RE_DUP_MAX) and mbsupport.h (to add a 
#define).

[0001-awk-simplify-dfa.c-by-having-it-not-include-mbsuppor.patch (text/plain, attachment)]

Message #64 received at 17157 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Aharon Robbins <arnold <at> skeeve.com>, 17381 <at> debbugs.gnu.org
Cc: 17072 <at> debbugs.gnu.org, 17157 <at> debbugs.gnu.org
Subject: Re: bug#17381: _GL_ATTRIBUTE_PURE in dfa.h
Date: Thu, 01 May 2014 23:11:06 -0700

[Message part 1 (text/plain, inline)]

On 05/01/2014 11:29 AM, Aharon Robbins wrote:
> custom.h is for system customization to override things that Autoconf 
> can't figure out or gets wrong

OK, it's easy to have something else include mbsupport.h instead. 
config.h, say.  The attached patch does that.  It doesn't really matter 
what includes it, so long as it's done before dfa.c and dfa.h start 
using the multibyte functions.

> Requiring gnulib in that header makes it less attractive to other 
> projects that might want to use dfa as a black box. Are there such? I 
> don't know. (I thought I'd heard something about gettext using dfa but 
> I am unsure if that is true.)

gettext uses gnulib, so that's not an issue.

> Does the GL_PURE stuff have to be on every declaration? Or can it just 
> be on the body?

It should be on the declaration for external functions, so that the 
function's caller knows to optimize it.

> What does it even mean

It means the function has no effects except the return value and that 
the return value depends only on the parameters and/or global variables.

> Does whatever optimization it enables *really* make a big difference, 
> or is it just a micro-optimization?

We put it in because GCC nowadays complains if we leave it out, if we 
configure with --enable-gcc-warnings.  The optimization seems to be a 
win in general and (more important) an aid for humans reading the code, 
so we typically just add the pure attribute and move on.

> Yes, I know. I am unsure if your patch, which totally eliminates the 
> ability to compile gawk on systems without multibyte support

It's not supposed to do that.  It's supposed to work on those hosts, by 
supplying substitutes for wchar_t, wctype_t, etc.  Hmm, are you worried 
about hosts that don't even have wchar.h and wctype.h?  If so, that can 
be worked around reasonably easily; please see attached patch.

> I just looked at the patch again. It really doesn't do the trick; 
> there are lots of places where MBS_SUPPORT is checked in the gawk code 
> and pulling mbsupport.h out of awk.h is likely to break things

No, it should still work.  With the revised patch, config.h includes 
mbsupport.h, so MBS_SUPPORT will be defined appropriately for gawk code 
and gawk's other MBS_SUPPORT usages will continue to work as before.

I'll CC: this to Bug#17157 and Bug#17072 as it's following up to the 
last messages in both those threads, too.

[0001-awk-simplify-dfa.c-by-having-it-not-include-mbsuppor.patch (text/plain, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.