On Friday 07 September 2007, Stepan Kasal wrote:
> I have reviewed your patch and noticed a bug:
> In the most common path, when SYS_something is set, the variable
> `syscall' is not set and thus the subsequent AM_CONDITIONAL is set
> incorrectly. (Symptom: on corrent computer, pivot_root is not build.)
i didnt use the variable "syscall" ... perhaps you mean "sysnum" ? if so,
yes, i see that now
> I suggest to add AC_REQUIRE([AC_CANONICAL_HOST]) to the beginning of
> the macro, because that is what provides $host_cpu. But that macro
> is already there, IIRC it is brought in AM_INIT_AUTOMAKE, so this
> does not cause problems in practice.
as you say, it shouldnt be needed in the normal case, but it certainly cant
hurt anything and if it were split off into a sep .m4, this change would only
be a good thing
> On Thu, Sep 06, 2007 at 08:09:06PM -0400, Mike Frysinger wrote:
> > On Thursday 06 September 2007, Mike Frysinger wrote:
> > one thing this is missing is cache handling ... but i dont know if you
> > really care that much ...
>
> The caching is important for ``inner'' macros, which may get called
> several times; then the subsequent calls do not repeat the expensive
> compiler calls. I agree it does not matter here...
>
> > so it shouldnt be too hard to integrate AC_CACHE_CHECK ...
>
> ... but OTOH it's really easy, so I added it. ;-)
>
> There is one thing you have to remember: the checking code may be
> skipped if the result is known from a cache. So the checking code
> may not call AC_DEFINE, AC_CONDITIONAL, or anything which sets the
> values for output. And, of course, none of the shell variables from
> the check can be used outside (in this case `syscall').
> All you have after AC_CACHE_CHECK is the ``cache variable'',
> util_cv_syscall_$1 in this case.
>
> But if you avoid this trap, AC_CACHE_CHECK actually helps: it calls
> AC_MSG_{CHECKING,RESULT} for you.
>
> Attached please find my variation on Mike's patch.
> Karel: Is there a way to declare two co-authors of a patch in git?
a quick review and things look nice, thanks
> + AM_CONDITIONAL([HAVE_]m4_toupper($1), [test $util_cv_syscall_$1 != no])
> + case $util_cv_syscall_$1 in #(
> + no) AC_MSG_WARN([Unable to detect syscall $1.]) ;;
> ...
> +[m4_ifval([$1],
> + [#(
> + $1) syscall="$2" ;;dnl
what are those #( for ?
-mike
Attachment:
signature.asc
Description: This is a digitally signed message part.
