bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: mdemo ltdl failure


From: Bruno Haible
Subject: Re: mdemo ltdl failure
Date: Wed, 25 Apr 2007 23:57:13 +0200
User-agent: KMail/1.5.4

Hi Ralf,

A bit of gnitpicking:

Ralf Wildenhues wrote:
> Here's what the gnulib patch looks like.

> +          set x `uname -r | $SED -e 's/^\([[0-9\.]]*\).*/\1/'`

$SED is usually not defined in the context of autoconf macros that are
part of gnulib. (I.e. it expands to empty.) Just use 'sed' instead.

Also, literals containing backslashes inside backquote cause trouble.
You normally need to write \\. instead of \. because of the outer
backquotes; only some shells (like bash) allow the use of \. inside backquote.
I recommend to assign this literal to a variable outside backquote. This
lso has the added benefit of documenting the meaning of the literal:

             sed_extract_leading_digits='s/^\([[0-9\.]]*\).*/\1/'
             set x `uname -r | sed -e "$sed_extract_leading_digits"`

> +        [case $host_os in #(
> +      *cygwin*)
> +        lt_cv_sys_argz_works=no
> +        if test "$cross_compiling" != no; then
> +          lt_cv_sys_argz_works="guessing no"
> +        else
> +          save_IFS=$IFS
> +          IFS=-.
> +          set x `uname -r | $SED -e 's/^\([[0-9\.]]*\).*/\1/'`
> +          IFS=$save_IFS
> +          lt_os_major=$[]{2-0}
> +          lt_os_minor=$[]{3-0}
> +          lt_os_micro=$[]{4-0}

This $[] is confusing. The entire shell code would be easier to understand
if you added a quoting level to all of it:

            [[case $host_os in #(
         *cygwin*)
           lt_cv_sys_argz_works=no
           if test "$cross_compiling" != no; then
             lt_cv_sys_argz_works="guessing no"
           else
             save_IFS=$IFS
             IFS=-.
             set x `uname -r | $SED -e 's/^\([0-9\.]*\).*/\1/'`
             IFS=$save_IFS
             lt_os_major=${2-0}
             lt_os_minor=${3-0}
             lt_os_micro=${4-0}

           ...
           ]]

> +          if test "$lt_os_major" -gt 1 ||
> +             { test "$lt_os_major" -eq 1 &&
> +               { test "$lt_os_minor" -gt 5 ||
> +                 { test "$lt_os_minor" -eq 5 &&
> +                   test "$lt_os_micro" -gt 24; }; }; }; then

The GNU standards ask, when breaking lines near an operator. for putting
the operator on the beginning of the second line, not at the end of the
first line.

> +        [AC_DEFINE([SYSTEM_ARGZ_IS_BROKEN], 1,
> +                   [This value is set to 1 to indicate that the system argz 
> facility does not work])

Most AC_DEFINEs have a positive sense rather than a negative one: can you
define a HAVE_WORKING_ARGZ in the opposite case, rather than this one?

Also, I don't see where in the C code this macro SYSTEM_ARGZ_IS_BROKEN is
being tested.

Bruno





reply via email to

[Prev in Thread] Current Thread [Next in Thread]