Ralf Wildenhues wrote:
* Charles Wilson wrote:
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.

Yes, I knew it was, which was why all "error" conditions in the test resulted in the test reporting *success*. I was hoping during discussion that somebody would have a better suggestion than "punt and use version numbers and explicit OS hacks, even tho that is not the Autoconf Way". But nobody commented on the patch until I raised it in private correspondence with BobF. Now, I just don't think there IS any way of actually detecting the error that isn't horrifyingly ugly.

And I felt really dirty after posting the original patch. :-)

(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.)

will do for linux & mingw, but have no unencumbered access to solaris at present.

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,

[EMAIL PROTECTED] and I checked to make sure those were fully supported by Posix Shell and weren't just convenient bashisms.

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.

Geez, solaris sux. You think they might consider shipping a posix compliant /bin/sh sometime this century?

+            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.

Okay, but actually, all cygwin releases are guaranteed to have non-empty major, minor, and micro numbers -- and we're inside a $host_os == cygwin block, here. If I update the os_* variable assignments with protection against empty (default to "0"?), I'll move it outside the case block so that if any other "bad" systems are discovered they can be added more easily -- although I doubt that there are any.

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.

Habit. I'll remove 'em all and see what breaks.

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.

That'll teach me to read the manual. I didn't realize it was n-ary until I did so, and thought: Cool! Never realizing I was reading the 2.61 manual...

--
Chuck


Reply via email to