Subject: Re: bug#17080: [PATCH] egrep, fgrep: go back to shell scripts

From: Paolo Bonzini <[email-masked]>
Date: Tue, 01 Apr 2014 10:11:58 +0200

Il 24/03/2014 01:21, Paul Eggert ha scritto:
> Although egrep's and fgrep's switch from shell scripts to
> executables may have made sense in 2005, it complicated
> maintenance and recently has caused subtle performance bugs.
> Go back to the old way of doing things, as it's simpler and more
> easily separated from the mainstream implementation.  This should
> be good enough nowadays, as POSIX has withdrawn egrep/fgrep and
> portable applications should be using -E/-F anyway.
> * po/POTFILES.in: Remove src/egrep.c, src/fgrep.c, src/main.c.
> * src/Makefile.am (bin_PROGRAMS): Remove egrep, fgrep.
> (bin_SCRIPTS): New macro.
> (grep_SOURCES): Move searchutils.c, dfa.c, dfasearch.c, kwset.c,
> kwsearch.c, pcresearch.c here from libgrep_a_SOURCES.
> (egrep_SOURCES, fgrep_SOURCES, noinst_LIBRARIES, libgrep_a_SOURCES):
> Remove.
> (LDADD): Remove libgrep.a.
> (egrep, fgrep): New rules.
> (CLEANFILES): New macro.
> * src/grep.c: Rename from src/main.c.
> (usage, setmatcher, main):
> Simplify, since there's now just one executable.
> (Gcompile, Ecompile, Acompile, GAcompile, PAcompile, matchers):
> Move here from the (removed) src/grep.c.
> (compile_fp_t, execute_fp_t, struct matcher, matchers):
> Move here from src/grep.h, as they no longer need to be public.
> (struct matcher.name): Avoid one level of indirection/relocation.
> (do_execute, main): Fix a performance bug when it was compiled
> as 'fgrep', due to confusion about which matcher was which.
> (main): Fix a performance bug with -P, likewise.
> * src/grep.h (before_options, after_options): Remove.
> * src/egrep.c, src/fgrep.c, src/grep.c: Remove.
> ---

I agree with Eli and Aharon that this change is unnecessary.

If you generate your patch with "-M" or add

	[diff]
	renames = true

to your .gitconfig file, the patch will also be more readable and it 
will be easier to see that it does not really introduce any simplifications.

Paolo