Re: mdemo ltdl failure

[Ralf Wildenhues]

Hi Charles,

* Charles Wilson wrote on Thu, Apr 19, 2007 at 03:02:08AM CEST:
>
> This is because the test is just too ugly for words, not to mention 
> brittle. Trying to tease out malloc issues outside of a dedicated malloc 
> testsuite is just plain silly.

I think the biggest problem with the previous patch was that it was
relying so much on undefined behavior, that is not only undefined by any
relevant standard, but also likely to be explicitly treated in very
unpredictable ways by different systems/compilers.

>> TESTING:
[...]
> Re-ran all of these tests under the specified conditions.  Same results as 
> the original patch.

Thanks.

>> (4) linux (whose system argz is OK)
>>     --detects success, builds using system argz, works.
>> (5) linux, but with $lt_cv_sys_argz_works=no.
>>     --doesn't run the test, and builds using libltdl argz. works.
>> (6) mingw, which doesn't have any system argz facility at all
>>     --the test is not run
>>     --builds successfully with libltdl's argz
>
> Did not run these tests. configury and sourcecode paths unchanged from 
> original path so these should obviously still work.  But I'll retest if 
> desired.

It's a good choice of testing, and should be done again with the final
patch.  Plus one test on Solaris with its /bin/sh.  (Just noting this,
I can probably do these tests then.)

Some comments to the patch:

> --- libltdl/m4/argz.m4        25 Mar 2006 11:05:02 -0000      1.3
> +++ libltdl/m4/argz.m4        17 Mar 2007 06:09:50 -0000
[...]
> +            os_ver=$(uname -r | $SED -e 's,^\([[0123456789\.]]*\).*,\1,')
> +            os_major=${os_ver%%\.*}
> +            os_micro=${os_ver##*\.}
> +            os_minor=${os_ver%\.*}
> +            os_minor=${os_minor##*\.}

You can not use the $(cmd) and the ${var%%pattern} constructs, Solaris'
shell will barf when parsing the code (i.e., even if it's inside a
conditional branch that is not executed).  Use `cmd` and sed or expr
instead, see the Autoconf manual's chapter on portable shell.

> +            AS_IF([test "$os_major" -gt "1"],[lt_cv_sys_argz_works=yes],dnl
> +                 [test "$os_minor" -gt "5"],[lt_cv_sys_argz_works=yes],dnl
> +                 [test "$os_micro" -gt "24"],[lt_cv_sys_argz_works=yes],dnl

No need for double quoting the literal integers.  And if it can happen
that $os_micro is the empty string (which I'll consider quite likely),
then double quoting isn't enough precaution against error output, as 
"" isn't interpreted by test as a number.

BTW, what are all the dnl's here for?  If they're necessary in some way,
that would indicate that AS_IF had a bug.  Ahh, that reminds me, AS_IF
has only been made n-ary in 2.60, and we intend to support 2.59.  Please
use plain `if..else' or nested AS_IF instead, thanks.

Cheers, and thank you for your work,
Ralf