autoconf-patches
[Top][All Lists]
Advanced

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

Re: document AS_BASENAME


From: Eric Blake
Subject: Re: document AS_BASENAME
Date: Sat, 01 Apr 2006 09:47:47 -0700
User-agent: Thunderbird 1.5 (Windows/20051201)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Stepan Kasal on 4/1/2006 8:35 AM:
> Hello,
> 
>> Thanks for spotting that bug, but there's a simpler patch, [...]
> 
> depends of the definition of simplicity.  Eric's point probably was
> that "expr ." is simpler than "expr . : '\(.\)'"

Exactly.  Unless there is an expr that mishandles strings unless you use
the : operator, then my patch was correct and produces a smaller configure.

> 
>>      * lib/m4sugar/m4sh.m4 (AS_BASENAME_EXPR): Handle ///, ////, etc.
>>      correctly.  Problem reported by Eric Blake.
> 
> I believe the real problem behind this is that we have two implementations 
> of basename: as an ``expr'' expression and as a Sed program, and each of
> them tries to handle all the corner cases.
> 
> I think it would be simpler to handle the corner cases once, by a shell
> ``case,'' and use expr||sed only for the non-trivial one.

I like the idea.  Nits below.

> @@ -649,12 +649,12 @@
>  ## 4. Portable versions of common tools.  ##
>  ## -------------------------------------- ##
>  
> -# This section is lexicographically sorted.
> -

Why not sort it?  I just posted a patch along this line in another mail.

>  m4_defun([AS_DIRNAME],
> -[(dirname $1) 2>/dev/null ||
> -AS_DIRNAME_EXPR([$1]) 2>/dev/null ||
> -AS_DIRNAME_SED([$1])])
> +[AS_REQUIRE([_AS_EXPR_PREPARE])dnl
> +AS_CASE([$1],
> +  [[*[!/]/*[!/]*]],

Are negated character classes portable?

> +    [(dirname $1) 2>/dev/null ||
> +     $as_expr X[]$1 : 'X\(.*[[^/]]\)//*[[^/][^/]]*/*$' 2>/dev/null ||
> +     echo X[]$1 | sed ['s|^X\(.*[^/]\)//*[^/][^/]*/*$|\1|p']],
> +  [[//|//[!/]*]],
> +    [(dirname $1) 2>/dev/null || echo //],

Save a process.  I would just blindly echo, rather than trying to use
dirname here.  After all, on systems where // is not special, it is still
a correct output, even if it doesn't match dirname.

> +  [/*], [echo /],
> +  [echo .])])
>  
>  
>  # AS_BASENAME(FILE-NAME)
>  # ----------------------
>  # Simulate the command 'basename FILE-NAME'.  Not all systems have basename.
> +# Try to respect the platform specific behavior for // and the empty string.
>  # Also see the comments for AS_DIRNAME.
> +AS_REQUIRE([_AS_EXPR_PREPARE])dnl
> +AS_CASE([$1],
> +  [[*[!/]*]],
> +    [$as_basename $1 ||
> +     $as_expr X/[]$1 : '.*/\([[^/][^/]*]\)/*$' 2>/dev/null ||
> +     echo X/[]$1 | sed ['s|^.*/\([^/][^/]*\)/*$|\1|']],
> +  [//], [$as_basename $1 || echo //],

Likewise - just echo // rather than trying basename.

> +  [''], [$as_basename $1 || echo .],

This bothers me a bit.  Normally, if "$file" does not exist, then 'cd
`dirname "$file"`; ls `basename "$file"`' will also fail.  But by making
the empty string evaluate to a valid filename, you are violating this
assumption.  GNU basename intentionally makes the empty string return the
empty string, for the safety factor of catching empty input filenames.

- --
Life is short - so eat dessert first!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFELq6z84KuGfSFAYARArXaAJ0XLIh6iZYj+uSqRNEujiuUnWYK9wCg06SD
d0/POBq4YySuSFdfwqePQ6A=
=JKYM
-----END PGP SIGNATURE-----




reply via email to

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