bug-grep
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: results after import of dfa into gawk


From: Jim Meyering
Subject: Re: results after import of dfa into gawk
Date: Fri, 19 Mar 2010 08:25:17 +0100

Aharon Robbins wrote:
> Hi. I just finished importing dfa.[hc] into gawk.

Good to know they're in sync.  Thanks.

> \s(20  THANK YOU !!! \s for all the work.  It passes make check. Attached
> is a diff from grep to gawk to retain additional things I need (e.g. VMS).
>
> FYI, I added more years in the copyright to dfa.c based on the years in
> which it was published as part of gawk.
>
> Please consider merging these into grep; they should not affect it and
> then we'll be 100% in sync.
>
> I can't thank the grep team enough for making this finally happen.

I know how you can thank us ;-) 1/2
Let us help you do the following:
 - switch gawk from cvs to git
 - make gawk use gnulib

Then (among other, more important benefits) it will be
easier/safer for gawk to stay in sync with grep's dfa.c.

> --- /usr/local/src/Gnu/grep/src/dfa.h 2010-03-17 21:56:16.000000000 +0200
> +++ dfa.h     2010-03-18 19:24:34.000000000 +0200
> @@ -350,7 +350,8 @@
>                                  -1 so we can count lines without wasting
>                                  too many cycles.  The transition for a
>                                  newline is stored separately and handled
> -                                as a special case. */
> +                                as a special case.  Newline is also used
> +                                as a sentinel at the end of the buffer. */
>    struct dfamust *musts;     /* List of strings, at least one of which
>                                  is known to appear in any r.e. matching
>                                  the dfa. */
> --- /usr/local/src/Gnu/grep/src/dfa.c 2010-03-17 21:56:16.000000000 +0200
> +++ dfa.c     2010-03-18 19:27:34.000000000 +0200
> @@ -1,5 +1,5 @@
>  /* dfa.c - deterministic extended regexp routines for GNU
> -   Copyright (C) 1988, 1998, 2000, 2002, 2004, 2008-2010 Free Software
> +   Copyright (C) 1988, 1998, 2000, 2002, 2004, 2005, 2007-2010 Free Software
>     Foundation, Inc.

I've made the above two changes with a commit in your name.
However, I'll pass on the other changes.

> +#ifdef HAVE_CONFIG_H
>  #include <config.h>
> +#endif

There's no point in using HAVE_CONFIG_H anymore.
In fact, we have a syntax-check rule that ensures new uses
don't sneak back in (consistency).  So the above would
trigger a test failure.

>  #include <assert.h>
>  #include <ctype.h>
>  #include <stdio.h>
> +
> +#ifndef VMS
>  #include <sys/types.h>
> +#else
>  #include <stddef.h>
> +#endif

Both sys/types.h and stddef.h are guaranteed to be available
and usable for grep, since it uses gnulib, so we might as well
include them unconditionally.

Besides, I suspect no one uses VMS anymore.

>  #include <stdlib.h>
>  #include <limits.h>
>  #include <string.h>
> -#include <locale.h>
> +#if HAVE_SETLOCALE
> +# include <locale.h>
> +#endif

Likewise.

> +#ifndef DEBUG        /* use the same approach as regex.c */
> +#undef assert
> +#define assert(e)
> +#endif /* DEBUG */

There is only one assertion, and it's trivial,
so I prefer to leave it enabled and to omit those lines.
Compile with NDEBUG if you want to turn off that sole assertion.

Already, enabling assertions caught at least one bug in recent review.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]