Re: [PATCHv2 2/9] getcwd-lgpl: new module

[Bruno Haible]

Hi Eric,

Now the code is clear enough that it can be understood :-) I have three remarks:

> +AC_DEFUN([gl_FUNC_GETCWD_LGPL],
> +[
> +  AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
> +  AC_REQUIRE([gl_FUNC_GETCWD_NULL])
> +
> +  case $gl_cv_func_getcwd_null in
> +  *yes) ;;
> +  *)
> +    dnl Minimal replacement
> +    REPLACE_GETCWD=1
> +    AC_LIBOBJ([getcwd-lgpl])
> +  esac

There is a missing ;; here, before the 'esac'.

> +      ptr = realloc (buf, size);
> +      if (ptr == NULL)
> +        {
> +          free (buf);
> +          errno = ENOMEM;
> +          return NULL;
> +        }
> +      result = getcwd (buf, size);

Here you ask getcwd() to fill a buffer at an address that does not exist
any more. The fix is to add a line "buf = ptr;":

      ptr = realloc (buf, size);
      if (ptr == NULL)
        {
          free (buf);
          errno = ENOMEM;
          return NULL;
        }
      buf = ptr;
      result = getcwd (buf, size);

>  AC_DEFUN([gl_PREREQ_GETCWD],
>  [
>    AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS])
>    AC_REQUIRE([gl_CHECK_TYPE_STRUCT_DIRENT_D_INO])
> +  AC_DEFINE([USE_GPL_GETCWD], [1],
> +      [Define to 1 if GPL code should be used to attempt to work around
> +       native getcwd failures.])

This will cause a link error if a package uses
"gnulib-tool --import --with-tests ... getcwd-lgpl ..."
and the module 'getcwd' occurs among the dependencies of the tests. Then
USE_GPL_GETCWD will be defined to 1, therefore lib/getcwd-lgpl.o will be empty,
therefore lib/libgnu.a will have a missing link dependency. (Recall that
lib/getcwd.o is linked into the tests only, not into lib/libgnu.a, in this
case.)

The easiest fix is to use
     gl_MODULE_INDICATOR([getcwd])
and then in the C code you use
     #if GNULIB_GETCWD

Another fix would be to change
     AC_DEFINE([USE_GPL_GETCWD], [1],
to
     AC_DEFINE([USE_GPL_GETCWD], [gl_MODULE_INDICATOR_CONDITION],
but that's not the common idiom: hardly anyone remembers what
gl_MODULE_INDICATOR_CONDITION stands for.

Bruno
-- 
In memoriam Heinrich Conradi <http://de.wikipedia.org/wiki/Heinrich_Conradi>