[PATCH 0/3] limit number of cat and sed forks

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 0/3] limit number of cat and sed forks

Paolo Bonzini-2
These three patches speed up libvirt's configure script by roughly 3%,
by avoiding repeated calls to sed (for AS_TR_SH and AS_TR_CPP, patches 1-2)
and cat (for _AC_DEFINE_UNQUOTED, patch 3).  The AS_TR_SH and AS_TR_CPP
computations are moved to autoconf time.  cat is replaced by printf.

Thanks,

Paolo

Paolo Bonzini (3):
  autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS
  autoconf: prefer an unrolled loop for trivial AC_CHECK_HEADERS
  autoconf: refine quadrigraph test in _AC_DEFINE_UNQUOTED

 lib/autoconf/functions.m4 | 25 +++++++++++--------------
 lib/autoconf/general.m4   |  6 ++++--
 lib/autoconf/headers.m4   | 30 +++++++++++++-----------------
 3 files changed, 28 insertions(+), 33 deletions(-)

--
2.7.4


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 3/3] autoconf: refine quadrigraph test in _AC_DEFINE_UNQUOTED

Paolo Bonzini-2
It is a very common case that a quadrigraph appears in the argument of
_AC_DEFINE_UNQUOTED, because "#define" is expanded through a quadrigraph.
Therefore, restrict the quadrigraph tests to "$" (for "$(" and "${")
and "(" (for "$(").

At the same time, "#" should not be used inside AC_ECHO because it confuses
m4's comment parsing.  This pre-existing latent bug is caught by test 251:

   AC_DEFINE_UNQUOTED([bar], [[%!_!# X]])

Previously the quadrigraph in "@%:@define bar %!_!# X" made Autoconf fall back
to cat anyway.  Now that Autoconf is not fooled by the quadrigraph, the test
catches that "#" is not special-cased.  Kudos to Eric for coming up with it!

* lib/autoconf/general (_AC_DEFINE_UNQUOTED): Do not blindly
use cat on all quadrigraphs.

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 lib/autoconf/general.m4 | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
index c15fb13..ef6285d 100644
--- a/lib/autoconf/general.m4
+++ b/lib/autoconf/general.m4
@@ -2167,9 +2167,11 @@ m4_define([AC_DEFINE_UNQUOTED], [_AC_DEFINE_Q([_$0], $@)])
 # Append the pre-expanded STRING and a newline to confdefs.h, as if
 # with an unquoted here-doc, but avoiding a fork in the common case of
 # no backslash, no command substitution, no complex variable
-# substitution, and no quadrigraphs.
+# substitution (taking into account quadrigraphs as well).  Also
+# avoid AS_ECHO if "#" is present to avoid confusing m4 with comments,
+# but quadrigraphs are fine in that case.
 m4_define([_AC_DEFINE_UNQUOTED],
-[m4_if(m4_bregexp([$1], [\\\|`\|\$(\|\${\|@]), [-1],
+[m4_if(m4_bregexp([$1], [#\|\\\|`\|\(\$\|@S|@\)\((|{|@{:@\)]), [-1],
        [AS_ECHO(["AS_ESCAPE([$1], [""])"]) >>confdefs.h],
        [cat >>confdefs.h <<_ACEOF
 [$1]
--
2.7.4


Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 2/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_HEADERS

Paolo Bonzini-2
In reply to this post by Paolo Bonzini-2
An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
AS_TR_CPP.  Prefer it if there is nothing in the second and third
argument of AC_CHECK_HEADERS and the first argument is a literal.
Modify AC_CHECK_HEADERS_ONCE to avoid the variable indirection too.

* lib/autoconf/headers.m4 (AC_CHECK_HEADERS): Unroll loop if safe.
(_AC_CHECK_HEADERS): Move basic implementation here.
(_AC_CHECK_HEADER_ONCE): Expand AC_CHECK_HEADERS here...
(_AC_HEADERS_EXPANSION): ... and not here, so remove.
(AC_CHECK_INCLUDES_DEFAULT): Remove unnecessary arguments after the first.

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 lib/autoconf/headers.m4 | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
index 72262c1..e855025 100644
--- a/lib/autoconf/headers.m4
+++ b/lib/autoconf/headers.m4
@@ -182,22 +182,25 @@ m4_define([AH_CHECK_HEADERS],
 # header.  Either ACTION may include `break' to stop the search.
 AC_DEFUN([AC_CHECK_HEADERS],
 [m4_map_args_w([$1], [_AH_CHECK_HEADER(], [)])]dnl
-[AS_FOR([AC_header], [ac_header], [$1],
-[AC_CHECK_HEADER(AC_header,
- [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]AC_header)) $2],
- [$3], [$4])dnl])
+[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
+       [m4_map_args_w([$1], [_$0(], [, [], [], [$4])])],
+       [AS_FOR([AC_header], [ac_header], [$1], [_$0(AC_header, [$2], [$3], [$4])])])
 ])# AC_CHECK_HEADERS
 
+m4_define([_AC_CHECK_HEADERS],
+[AC_CHECK_HEADER([$1],
+ [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]$1)) $2],
+ [$3], [$4])
+])
+
 
 # _AC_CHECK_HEADER_ONCE(HEADER-FILE)
 # ----------------------------------
 # Check for a single HEADER-FILE once.
 m4_define([_AC_CHECK_HEADER_ONCE],
-[_AH_CHECK_HEADER([$1])AC_DEFUN([_AC_Header_]m4_translit([[$1]],
-    [./-], [___]),
-  [m4_divert_text([INIT_PREPARE], [AS_VAR_APPEND([ac_header_list], [" $1"])])
-_AC_HEADERS_EXPANSION])AC_REQUIRE([_AC_Header_]m4_translit([[$1]],
-    [./-], [___]))])
+[AC_DEFUN([_AC_Header_]m4_translit([[$1]], [./-], [___]),
+          [AC_CHECK_HEADERS([$1], [], [], [$ac_includes_default])])
+AC_REQUIRE([_AC_Header_]m4_translit([[$1]], [./-], [___]))])
 
 
 # AC_CHECK_HEADERS_ONCE(HEADER-FILE...)
@@ -213,12 +216,6 @@ AC_DEFUN([AC_CHECK_HEADERS_ONCE],
 AC_DEFUN([_AC_CHECK_HEADERS_ONCE],
   [m4_map_args_w([$1], [_AC_CHECK_HEADER_ONCE(], [)])])
 
-m4_define([_AC_HEADERS_EXPANSION],
-  [m4_divert_text([DEFAULTS], [ac_header_list=])]dnl
-  [AC_CHECK_HEADERS([$ac_header_list], [], [], [$ac_includes_default])]dnl
-  [m4_define([_AC_HEADERS_EXPANSION], [])])
-
-
 
 
 ## --------------------- ##
@@ -261,8 +258,7 @@ ac_includes_default="\
 #endif"
 ])]dnl
 [_AC_CHECK_HEADERS_ONCE(
-  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h],
-  [], [], [$ac_includes_default])]dnl
+  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h])]dnl
 dnl For backward compatibility, provide unconditional AC_DEFINEs of
 dnl HAVE_STDLIB_H, HAVE_STRING_H, and STDC_HEADERS.
 [AC_DEFINE([HAVE_STDLIB_H], [1],
--
2.7.4



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

Paolo Bonzini-2
In reply to this post by Paolo Bonzini-2
An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
AS_TR_CPP.  Prefer it if there is nothing in the second and third
argument of AC_CHECK_FUNCS and the first argument is a literal.
Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.

* lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
(_AC_CHECK_FUNCS): Move basic implementation here.
(_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
(_AC_FUNCS_EXPANSION): ... and not here, so remove.

Signed-off-by: Paolo Bonzini <[hidden email]>
---
 lib/autoconf/functions.m4 | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index 66abe29..1043192 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -85,20 +85,24 @@ m4_define([_AH_CHECK_FUNC],
 # `break' to stop the search.
 AC_DEFUN([AC_CHECK_FUNCS],
 [m4_map_args_w([$1], [_AH_CHECK_FUNC(], [)])]dnl
-[AS_FOR([AC_func], [ac_func], [$1],
-[AC_CHECK_FUNC(AC_func,
-       [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]AC_func)) $2],
-       [$3])dnl])
+[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
+       [m4_map_args_w([$1], [_$0(], [)])],
+       [AS_FOR([AC_func], [ac_func], [$1], [_$0(AC_func, [$2], [$3])])])
 ])# AC_CHECK_FUNCS
 
+m4_define([_AC_CHECK_FUNCS],
+[AC_CHECK_FUNC([$1],
+       [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]$1)) $2],
+       [$3])
+])
+
 
 # _AC_CHECK_FUNC_ONCE(FUNCTION)
 # -----------------------------
 # Check for a single FUNCTION once.
 m4_define([_AC_CHECK_FUNC_ONCE],
-[_AH_CHECK_FUNC([$1])AC_DEFUN([_AC_Func_$1],
-  [m4_divert_text([INIT_PREPARE], [AS_VAR_APPEND([ac_func_list], [" $1"])])
-_AC_FUNCS_EXPANSION])AC_REQUIRE([_AC_Func_$1])])
+[AC_DEFUN([_AC_Func_$1], [AC_CHECK_FUNCS([$1])])
+AC_REQUIRE([_AC_Func_$1])])
 
 # AC_CHECK_FUNCS_ONCE(FUNCTION...)
 # --------------------------------
@@ -107,13 +111,6 @@ _AC_FUNCS_EXPANSION])AC_REQUIRE([_AC_Func_$1])])
 AC_DEFUN([AC_CHECK_FUNCS_ONCE],
 [m4_map_args_w([$1], [_AC_CHECK_FUNC_ONCE(], [)])])
 
-m4_define([_AC_FUNCS_EXPANSION],
-[
-  m4_divert_text([DEFAULTS], [ac_func_list=])
-  AC_CHECK_FUNCS([$ac_func_list])
-  m4_define([_AC_FUNCS_EXPANSION], [])
-])
-
 
 # _AC_REPLACE_FUNC(FUNCTION)
 # --------------------------
--
2.7.4



Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

Eric Blake-3
On 10/31/2016 06:39 AM, Paolo Bonzini wrote:

> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
> AS_TR_CPP.  Prefer it if there is nothing in the second and third
> argument of AC_CHECK_FUNCS and the first argument is a literal.
> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
>
> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
> (_AC_CHECK_FUNCS): Move basic implementation here.
> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
>
> Signed-off-by: Paolo Bonzini <[hidden email]>
> ---
>  lib/autoconf/functions.m4 | 25 +++++++++++--------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
ACK

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

Eric Blake-3
In reply to this post by Paolo Bonzini-2
On 10/31/2016 06:39 AM, Paolo Bonzini wrote:
> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
> AS_TR_CPP.  Prefer it if there is nothing in the second and third
> argument of AC_CHECK_FUNCS and the first argument is a literal.
> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
>
> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
> (_AC_CHECK_FUNCS): Move basic implementation here.
> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
> (_AC_FUNCS_EXPANSION): ... and not here, so remove.

> -       [$3])dnl])
> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,

Why []yes instead of the more typical [yes] ?

If the user has (unwisely) defined yes as a macro, your version will
compare against their expansion, instead of against the intended literal.

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

Paolo Bonzini-2


On 31/10/2016 16:28, Eric Blake wrote:

> On 10/31/2016 06:39 AM, Paolo Bonzini wrote:
>> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
>> AS_TR_CPP.  Prefer it if there is nothing in the second and third
>> argument of AC_CHECK_FUNCS and the first argument is a literal.
>> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
>>
>> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
>> (_AC_CHECK_FUNCS): Move basic implementation here.
>> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
>> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
>
>> -       [$3])dnl])
>> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
>
> Why []yes instead of the more typical [yes] ?
>
> If the user has (unwisely) defined yes as a macro, your version will
> compare against their expansion, instead of against the intended literal.
Because I've never understood the rules for m4_if, and thought []yes
matched what you get from m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]).

Paolo


signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 2/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_HEADERS

Eric Blake-3
In reply to this post by Paolo Bonzini-2
On 10/31/2016 06:39 AM, Paolo Bonzini wrote:

> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
> AS_TR_CPP.  Prefer it if there is nothing in the second and third
> argument of AC_CHECK_HEADERS and the first argument is a literal.
> Modify AC_CHECK_HEADERS_ONCE to avoid the variable indirection too.
>
> * lib/autoconf/headers.m4 (AC_CHECK_HEADERS): Unroll loop if safe.
> (_AC_CHECK_HEADERS): Move basic implementation here.
> (_AC_CHECK_HEADER_ONCE): Expand AC_CHECK_HEADERS here...
> (_AC_HEADERS_EXPANSION): ... and not here, so remove.
> (AC_CHECK_INCLUDES_DEFAULT): Remove unnecessary arguments after the first.
>
> Signed-off-by: Paolo Bonzini <[hidden email]>
> ---
>  lib/autoconf/headers.m4 | 30 +++++++++++++-----------------
>  1 file changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
> index 72262c1..e855025 100644
> --- a/lib/autoconf/headers.m4
> +++ b/lib/autoconf/headers.m4
> @@ -182,22 +182,25 @@ m4_define([AH_CHECK_HEADERS],
>  # header.  Either ACTION may include `break' to stop the search.
>  AC_DEFUN([AC_CHECK_HEADERS],
>  [m4_map_args_w([$1], [_AH_CHECK_HEADER(], [)])]dnl
> -[AS_FOR([AC_header], [ac_header], [$1],
> -[AC_CHECK_HEADER(AC_header,
> - [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]AC_header)) $2],
> - [$3], [$4])dnl])
> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
Again, why []yes instead of [yes]?

>  
>  ## --------------------- ##
> @@ -261,8 +258,7 @@ ac_includes_default="\
>  #endif"
>  ])]dnl
>  [_AC_CHECK_HEADERS_ONCE(
> -  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h],
> -  [], [], [$ac_includes_default])]dnl
> +  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h])]dnl

This one, I'm worried about.  $ac_includes_default is a shell variable
not documented in the manual (so we can probably get away with changing
it), but doesn't it exist so that projects can redefine the core set of
default headers to check by default, whereas you are now losing that
flexibility?

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 2/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_HEADERS

Paolo Bonzini-2


On 31/10/2016 16:32, Eric Blake wrote:

> On 10/31/2016 06:39 AM, Paolo Bonzini wrote:
>> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
>> AS_TR_CPP.  Prefer it if there is nothing in the second and third
>> argument of AC_CHECK_HEADERS and the first argument is a literal.
>> Modify AC_CHECK_HEADERS_ONCE to avoid the variable indirection too.
>>
>> * lib/autoconf/headers.m4 (AC_CHECK_HEADERS): Unroll loop if safe.
>> (_AC_CHECK_HEADERS): Move basic implementation here.
>> (_AC_CHECK_HEADER_ONCE): Expand AC_CHECK_HEADERS here...
>> (_AC_HEADERS_EXPANSION): ... and not here, so remove.
>> (AC_CHECK_INCLUDES_DEFAULT): Remove unnecessary arguments after the first.
>>
>> Signed-off-by: Paolo Bonzini <[hidden email]>
>> ---
>>  lib/autoconf/headers.m4 | 30 +++++++++++++-----------------
>>  1 file changed, 13 insertions(+), 17 deletions(-)
>>
>> diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
>> index 72262c1..e855025 100644
>> --- a/lib/autoconf/headers.m4
>> +++ b/lib/autoconf/headers.m4
>> @@ -182,22 +182,25 @@ m4_define([AH_CHECK_HEADERS],
>>  # header.  Either ACTION may include `break' to stop the search.
>>  AC_DEFUN([AC_CHECK_HEADERS],
>>  [m4_map_args_w([$1], [_AH_CHECK_HEADER(], [)])]dnl
>> -[AS_FOR([AC_header], [ac_header], [$1],
>> -[AC_CHECK_HEADER(AC_header,
>> - [AC_DEFINE_UNQUOTED(AS_TR_CPP([HAVE_]AC_header)) $2],
>> - [$3], [$4])dnl])
>> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
>
> Again, why []yes instead of [yes]?
>
>>  
>>  ## --------------------- ##
>> @@ -261,8 +258,7 @@ ac_includes_default="\
>>  #endif"
>>  ])]dnl
>>  [_AC_CHECK_HEADERS_ONCE(
>> -  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h],
>> -  [], [], [$ac_includes_default])]dnl
>> +  [sys/types.h sys/stat.h strings.h inttypes.h stdint.h unistd.h])]dnl
>
> This one, I'm worried about.  $ac_includes_default is a shell variable
> not documented in the manual (so we can probably get away with changing
> it), but doesn't it exist so that projects can redefine the core set of
> default headers to check by default, whereas you are now losing that
> flexibility?
No, the arguments after the first were unused:

AC_DEFUN([_AC_CHECK_HEADERS_ONCE],
  [m4_map_args_w([$1], [_AC_CHECK_HEADER_ONCE(], [)])])


The variable is still used in my patch by _AC_CHECK_HEADER_ONCE:

m4_define([_AC_CHECK_HEADER_ONCE],
[AC_DEFUN([_AC_Header_]m4_translit([[$1]], [./-], [___]),
          [AC_CHECK_HEADERS([$1], [], [], [$ac_includes_default])])
AC_REQUIRE([_AC_Header_]m4_translit([[$1]], [./-], [___]))])


In fact, the whole usage of $ac_includes_default is only there to avoid
recursion between AC_INCLUDES_DEFAULT and AC_CHECK_HEADERS_ONCE: instead
of using AC_INCLUDES_DEFAULT as the fourth argument, we include its
expansion directly.

Now, because AC_CHECK_INCLUDES_DEFAULT is AC_DEFUN_ONCE'd, the mutual
recursion you would not produce an _infinite recursion_; however, you
would still get a bogus warning about m4_requiring a macro that is not
m4_defun'd.

Thanks,

Paolo


signature.asc (484 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 3/3] autoconf: refine quadrigraph test in _AC_DEFINE_UNQUOTED

Eric Blake-3
In reply to this post by Paolo Bonzini-2
On 10/31/2016 06:39 AM, Paolo Bonzini wrote:

> It is a very common case that a quadrigraph appears in the argument of
> _AC_DEFINE_UNQUOTED, because "#define" is expanded through a quadrigraph.
> Therefore, restrict the quadrigraph tests to "$" (for "$(" and "${")
> and "(" (for "$(").
>
> At the same time, "#" should not be used inside AC_ECHO because it confuses
> m4's comment parsing.  This pre-existing latent bug is caught by test 251:
>
>    AC_DEFINE_UNQUOTED([bar], [[%!_!# X]])
>
> Previously the quadrigraph in "@%:@define bar %!_!# X" made Autoconf fall back
> to cat anyway.  Now that Autoconf is not fooled by the quadrigraph, the test
> catches that "#" is not special-cased.  Kudos to Eric for coming up with it!
>
> * lib/autoconf/general (_AC_DEFINE_UNQUOTED): Do not blindly
> use cat on all quadrigraphs.
>
> Signed-off-by: Paolo Bonzini <[hidden email]>
> ---
>  lib/autoconf/general.m4 | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
ACK.

>
> diff --git a/lib/autoconf/general.m4 b/lib/autoconf/general.m4
> index c15fb13..ef6285d 100644
> --- a/lib/autoconf/general.m4
> +++ b/lib/autoconf/general.m4
> @@ -2167,9 +2167,11 @@ m4_define([AC_DEFINE_UNQUOTED], [_AC_DEFINE_Q([_$0], $@)])
>  # Append the pre-expanded STRING and a newline to confdefs.h, as if
>  # with an unquoted here-doc, but avoiding a fork in the common case of
>  # no backslash, no command substitution, no complex variable
> -# substitution, and no quadrigraphs.
> +# substitution (taking into account quadrigraphs as well).  Also
> +# avoid AS_ECHO if "#" is present to avoid confusing m4 with comments,
> +# but quadrigraphs are fine in that case.
>  m4_define([_AC_DEFINE_UNQUOTED],
> -[m4_if(m4_bregexp([$1], [\\\|`\|\$(\|\${\|@]), [-1],
> +[m4_if(m4_bregexp([$1], [#\|\\\|`\|\(\$\|@S|@\)\((|{|@{:@\)]), [-1],
Line noise for the win!

>         [AS_ECHO(["AS_ESCAPE([$1], [""])"]) >>confdefs.h],
>         [cat >>confdefs.h <<_ACEOF
>  [$1]
>

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

Eric Blake-3
In reply to this post by Paolo Bonzini-2
On 10/31/2016 10:33 AM, Paolo Bonzini wrote:

>
>
> On 31/10/2016 16:28, Eric Blake wrote:
>> On 10/31/2016 06:39 AM, Paolo Bonzini wrote:
>>> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
>>> AS_TR_CPP.  Prefer it if there is nothing in the second and third
>>> argument of AC_CHECK_FUNCS and the first argument is a literal.
>>> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
>>>
>>> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
>>> (_AC_CHECK_FUNCS): Move basic implementation here.
>>> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
>>> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
>>
>>> -       [$3])dnl])
>>> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
>>
>> Why []yes instead of the more typical [yes] ?
>>
>> If the user has (unwisely) defined yes as a macro, your version will
>> compare against their expansion, instead of against the intended literal.
>
> Because I've never understood the rules for m4_if, and thought []yes
> matched what you get from m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]).
If 'yes' and 'no' are not macros, then these are identical:

m4_if([$2$3]AS_LITERAL_IF([$1], yes, no), yes, ...)
m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes[], ...)
m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes], ...)

because of the order in which quotes are stripped through successive
levels of m4 processing.  But if yes or no could be a macro, then only
the latter form is safe against unintended expansions (yes, including
the double-quoting within AS_LITERAL_IF, because one layer of quotes
gets stripped when collecting the args to AS_LITERAL_IF, and another
gets stripped when concatenating to [$2$3] to determine the final string
to compare against the literal of the second argument to m4_if).

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


signature.asc (617 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [PATCH 1/3] autoconf: prefer an unrolled loop for trivial AC_CHECK_FUNCS

Paolo Bonzini-2


On 31/10/2016 18:01, Eric Blake wrote:

> On 10/31/2016 10:33 AM, Paolo Bonzini wrote:
>>
>>
>> On 31/10/2016 16:28, Eric Blake wrote:
>>> On 10/31/2016 06:39 AM, Paolo Bonzini wrote:
>>>> An unrolled loop avoids the cost of spawning sed in AS_TR_SH and
>>>> AS_TR_CPP.  Prefer it if there is nothing in the second and third
>>>> argument of AC_CHECK_FUNCS and the first argument is a literal.
>>>> Modify AC_CHECK_FUNCS_ONCE to avoid the variable indirection too.
>>>>
>>>> * lib/autoconf/functions.m4 (AC_CHECK_FUNCS): Unroll loop if safe.
>>>> (_AC_CHECK_FUNCS): Move basic implementation here.
>>>> (_AC_CHECK_FUNC_ONCE): Expand AC_CHECK_FUNCS here...
>>>> (_AC_FUNCS_EXPANSION): ... and not here, so remove.
>>>
>>>> -       [$3])dnl])
>>>> +[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
>>>
>>> Why []yes instead of the more typical [yes] ?
>>>
>>> If the user has (unwisely) defined yes as a macro, your version will
>>> compare against their expansion, instead of against the intended literal.
>>
>> Because I've never understood the rules for m4_if, and thought []yes
>> matched what you get from m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]).
>
> If 'yes' and 'no' are not macros, then these are identical:
>
> m4_if([$2$3]AS_LITERAL_IF([$1], yes, no), yes, ...)
> m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes[], ...)
> m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes], ...)
>
> because of the order in which quotes are stripped through successive
> levels of m4 processing.  But if yes or no could be a macro, then only
> the latter form is safe against unintended expansions (yes, including
> the double-quoting within AS_LITERAL_IF, because one layer of quotes
> gets stripped when collecting the args to AS_LITERAL_IF, and another
> gets stripped when concatenating to [$2$3] to determine the final string
> to compare against the literal of the second argument to m4_if).
Good, this is what I wanted.  I'll send v2 where the
only difference will be:

diff --git a/lib/autoconf/functions.m4 b/lib/autoconf/functions.m4
index 1043192..ccfb053 100644
--- a/lib/autoconf/functions.m4
+++ b/lib/autoconf/functions.m4
@@ -85,7 +85,7 @@ m4_define([_AH_CHECK_FUNC],
 # `break' to stop the search.
 AC_DEFUN([AC_CHECK_FUNCS],
 [m4_map_args_w([$1], [_AH_CHECK_FUNC(], [)])]dnl
-[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
+[m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes],
        [m4_map_args_w([$1], [_$0(], [)])],
        [AS_FOR([AC_func], [ac_func], [$1], [_$0(AC_func, [$2], [$3])])])
 ])# AC_CHECK_FUNCS
diff --git a/lib/autoconf/headers.m4 b/lib/autoconf/headers.m4
index e855025..42f373f 100644
--- a/lib/autoconf/headers.m4
+++ b/lib/autoconf/headers.m4
@@ -182,7 +182,7 @@ m4_define([AH_CHECK_HEADERS],
 # header.  Either ACTION may include `break' to stop the search.
 AC_DEFUN([AC_CHECK_HEADERS],
 [m4_map_args_w([$1], [_AH_CHECK_HEADER(], [)])]dnl
-[m4_if([$2$3]AS_LITERAL_IF([$1], [yes], [no]), []yes,
+[m4_if([$2$3]AS_LITERAL_IF([$1], [[yes]], [[no]]), [yes],
        [m4_map_args_w([$1], [_$0(], [, [], [], [$4])])],
        [AS_FOR([AC_header], [ac_header], [$1], [_$0(AC_header, [$2], [$3], [$4])])])
 ])# AC_CHECK_HEADERS


Please commit for me, since I don't have write access to autoconf.git.

Thanks,

Paolo


signature.asc (484 bytes) Download Attachment
Loading...