diff mbox series

[ovs-dev,1/2] acinclude: Add support for grep option.

Message ID 1508164005-57189-1-git-send-email-u9012063@gmail.com
State Changes Requested
Headers show
Series [ovs-dev,1/2] acinclude: Add support for grep option. | expand

Commit Message

William Tu Oct. 16, 2017, 2:26 p.m. UTC
Allow to pass grep's option to OVS_GREP_IFELSE.
One use case is to pass '-w' for exact match.

Signed-off-by: William Tu <u9012063@gmail.com>
---
 acinclude.m4 | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Gregory Rose Oct. 18, 2017, 10:33 p.m. UTC | #1
On 10/16/2017 07:26 AM, William Tu wrote:
> Allow to pass grep's option to OVS_GREP_IFELSE.
> One use case is to pass '-w' for exact match.
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>   acinclude.m4 | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 89f88ca8de75..543e1e3f0630 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -326,15 +326,15 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>     AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
>   ])
>   
> -dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
> +dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH], [OPTION])
>   dnl
> -dnl Greps FILE for REGEX.  If it matches, runs IF-MATCH, otherwise IF-NO-MATCH.
> -dnl If IF-MATCH is empty then it defines to OVS_DEFINE(HAVE_<REGEX>), with
> -dnl <REGEX> translated to uppercase.
> +dnl Greps FILE for REGEX with grep's OPTION.  If it matches, runs IF-MATCH,
> +dnl otherwise IF-NO-MATCH.  If IF-MATCH is empty then it defines to
> +dnl OVS_DEFINE(HAVE_<REGEX>), with <REGEX> translated to uppercase.
>   AC_DEFUN([OVS_GREP_IFELSE], [
>     AC_MSG_CHECKING([whether $2 matches in $1])
>     if test -f $1; then
> -    grep '$2' $1 >/dev/null 2>&1
> +    grep $5 '$2' $1 >/dev/null 2>&1
>       status=$?
>       case $status in
>         0)
> 

LGTM

Tested-by: Greg Rose <gvrose8192@gmail.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Ben Pfaff Oct. 30, 2017, 8:04 p.m. UTC | #2
On Mon, Oct 16, 2017 at 07:26:44AM -0700, William Tu wrote:
> Allow to pass grep's option to OVS_GREP_IFELSE.
> One use case is to pass '-w' for exact match.
> 
> Signed-off-by: William Tu <u9012063@gmail.com>

POSIX doesn't mention a -w option, and the Autoconf manual says that it
is not portable in practice.  It also says that \b is not portable in
practice.

Is there another way to accomplish what you want to do?  For example,
how about HAVE_SKB_GSO_UDP[^_]?  Since this is Autoconf, probably it's
necessary to double the [], as: HAVE_SKB_GSO_UDP[[^_]]

(We don't really care that much about portability to everything that
Autoconf supports, so probably we could really use -w or \b in
practice.)

This is what the Autoconf manual says:

'grep'
     Portable scripts can rely on the 'grep' options '-c', '-l', '-n',
     and '-v', but should avoid other options.  For example, don't use
     '-w', as Posix does not require it and Irix 6.5.16m's 'grep' does
     not support it.  Also, portable scripts should not combine '-c'
     with '-l', as Posix does not allow this.

     Some of the options required by Posix are not portable in practice.
     Don't use 'grep -q' to suppress output, because many 'grep'
     implementations (e.g., Solaris) do not support '-q'.  Don't use
     'grep -s' to suppress output either, because Posix says '-s' does
     not suppress output, only some error messages; also, the '-s'
     option of traditional 'grep' behaved like '-q' does in most modern
     implementations.  Instead, redirect the standard output and
     standard error (in case the file doesn't exist) of 'grep' to
     '/dev/null'.  Check the exit status of 'grep' to determine whether
     it found a match.

     The QNX4 implementation fails to count lines with 'grep -c '$'',
     but works with 'grep -c '^''.  Other alternatives for counting
     lines are to use 'sed -n '$='' or 'wc -l'.

     Some traditional 'grep' implementations do not work on long input
     lines.  On AIX the default 'grep' silently truncates long lines on
     the input before matching.

     Also, many implementations do not support multiple regexps with
     '-e': they either reject '-e' entirely (e.g., Solaris) or honor
     only the last pattern (e.g., IRIX 6.5 and NeXT). To work around
     these problems, invoke 'AC_PROG_GREP' and then use '$GREP'.

     Another possible workaround for the multiple '-e' problem is to
     separate the patterns by newlines, for example:

          grep 'foo
          bar' in.txt

     except that this fails with traditional 'grep' implementations and
     with OpenBSD 3.8 'grep'.

     Traditional 'grep' implementations (e.g., Solaris) do not support
     the '-E' or '-F' options.  To work around these problems, invoke
     'AC_PROG_EGREP' and then use '$EGREP', and similarly for
     'AC_PROG_FGREP' and '$FGREP'.  Even if you are willing to require
     support for Posix 'grep', your script should not use both '-E' and
     '-F', since Posix does not allow this combination.

     Portable 'grep' regular expressions should use '\' only to escape
     characters in the string '$()*.0123456789[\^{}'.  For example,
     alternation, '\|', is common but Posix does not require its support
     in basic regular expressions, so it should be avoided in portable
     scripts.  Solaris and HP-UX 'grep' do not support it.  Similarly,
     the following escape sequences should also be avoided: '\<', '\>',
     '\+', '\?', '\`', '\'', '\B', '\b', '\S', '\s', '\W', and '\w'.

     Posix does not specify the behavior of 'grep' on binary files.  An
     example where this matters is using BSD 'grep' to search text that
     includes embedded ANSI escape sequences for colored output to
     terminals ('\033[m' is the sequence to restore normal output); the
     behavior depends on whether input is seekable:

          $ printf 'esc\033[mape\n' > sample
          $ grep . sample
          Binary file sample matches
          $ cat sample | grep .
          escape
William Tu Oct. 31, 2017, 10:36 p.m. UTC | #3
On Mon, Oct 30, 2017 at 1:04 PM, Ben Pfaff <blp@ovn.org> wrote:
> On Mon, Oct 16, 2017 at 07:26:44AM -0700, William Tu wrote:
>> Allow to pass grep's option to OVS_GREP_IFELSE.
>> One use case is to pass '-w' for exact match.
>>
>> Signed-off-by: William Tu <u9012063@gmail.com>
>
> POSIX doesn't mention a -w option, and the Autoconf manual says that it
> is not portable in practice.  It also says that \b is not portable in
> practice.
>
> Is there another way to accomplish what you want to do?  For example,
> how about HAVE_SKB_GSO_UDP[^_]?  Since this is Autoconf, probably it's
> necessary to double the [], as: HAVE_SKB_GSO_UDP[[^_]]
>
> (We don't really care that much about portability to everything that
> Autoconf supports, so probably we could really use -w or \b in
> practice.)
>
> This is what the Autoconf manual says:
>
> 'grep'
>      Portable scripts can rely on the 'grep' options '-c', '-l', '-n',
>      and '-v', but should avoid other options.  For example, don't use
>      '-w', as Posix does not require it and Irix 6.5.16m's 'grep' does
>      not support it.  Also, portable scripts should not combine '-c'
>      with '-l', as Posix does not allow this.
>
>      Some of the options required by Posix are not portable in practice.
>      Don't use 'grep -q' to suppress output, because many 'grep'
>      implementations (e.g., Solaris) do not support '-q'.  Don't use
>      'grep -s' to suppress output either, because Posix says '-s' does
>      not suppress output, only some error messages; also, the '-s'
>      option of traditional 'grep' behaved like '-q' does in most modern
>      implementations.  Instead, redirect the standard output and
>      standard error (in case the file doesn't exist) of 'grep' to
>      '/dev/null'.  Check the exit status of 'grep' to determine whether
>      it found a match.
>
>      The QNX4 implementation fails to count lines with 'grep -c '$'',
>      but works with 'grep -c '^''.  Other alternatives for counting
>      lines are to use 'sed -n '$='' or 'wc -l'.
>
>      Some traditional 'grep' implementations do not work on long input
>      lines.  On AIX the default 'grep' silently truncates long lines on
>      the input before matching.
>
>      Also, many implementations do not support multiple regexps with
>      '-e': they either reject '-e' entirely (e.g., Solaris) or honor
>      only the last pattern (e.g., IRIX 6.5 and NeXT). To work around
>      these problems, invoke 'AC_PROG_GREP' and then use '$GREP'.
>
>      Another possible workaround for the multiple '-e' problem is to
>      separate the patterns by newlines, for example:
>
>           grep 'foo
>           bar' in.txt
>
>      except that this fails with traditional 'grep' implementations and
>      with OpenBSD 3.8 'grep'.
>
>      Traditional 'grep' implementations (e.g., Solaris) do not support
>      the '-E' or '-F' options.  To work around these problems, invoke
>      'AC_PROG_EGREP' and then use '$EGREP', and similarly for
>      'AC_PROG_FGREP' and '$FGREP'.  Even if you are willing to require
>      support for Posix 'grep', your script should not use both '-E' and
>      '-F', since Posix does not allow this combination.
>
>      Portable 'grep' regular expressions should use '\' only to escape
>      characters in the string '$()*.0123456789[\^{}'.  For example,
>      alternation, '\|', is common but Posix does not require its support
>      in basic regular expressions, so it should be avoided in portable
>      scripts.  Solaris and HP-UX 'grep' do not support it.  Similarly,
>      the following escape sequences should also be avoided: '\<', '\>',
>      '\+', '\?', '\`', '\'', '\B', '\b', '\S', '\s', '\W', and '\w'.
>
>      Posix does not specify the behavior of 'grep' on binary files.  An
>      example where this matters is using BSD 'grep' to search text that
>      includes embedded ANSI escape sequences for colored output to
>      terminals ('\033[m' is the sequence to restore normal output); the
>      behavior depends on whether input is seekable:
>
>           $ printf 'esc\033[mape\n' > sample
>           $ grep . sample
>           Binary file sample matches
>           $ cat sample | grep .
>           escape

Thanks for the feedback. I will submit v2 patch to use
HAVE_SKB_GSO_UDP[[^_]] instead.
William
diff mbox series

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index 89f88ca8de75..543e1e3f0630 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -326,15 +326,15 @@  AC_DEFUN([OVS_CHECK_DPDK], [
   AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true)
 ])
 
-dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
+dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH], [OPTION])
 dnl
-dnl Greps FILE for REGEX.  If it matches, runs IF-MATCH, otherwise IF-NO-MATCH.
-dnl If IF-MATCH is empty then it defines to OVS_DEFINE(HAVE_<REGEX>), with
-dnl <REGEX> translated to uppercase.
+dnl Greps FILE for REGEX with grep's OPTION.  If it matches, runs IF-MATCH,
+dnl otherwise IF-NO-MATCH.  If IF-MATCH is empty then it defines to
+dnl OVS_DEFINE(HAVE_<REGEX>), with <REGEX> translated to uppercase.
 AC_DEFUN([OVS_GREP_IFELSE], [
   AC_MSG_CHECKING([whether $2 matches in $1])
   if test -f $1; then
-    grep '$2' $1 >/dev/null 2>&1
+    grep $5 '$2' $1 >/dev/null 2>&1
     status=$?
     case $status in
       0)