Re: Patch for libpst (for review)

[Guillem Jover <guillem@debian.org>]

Hi!

On Wed, 2012-06-20 at 19:10:17 +0200, Svante Signell wrote:
> If there is a need to build pst2dii that .cpp-file has to be patched too
> (not done yet). Looks like it will be enabled if the right version
> of /usr/bin/convert is found (probably from imagemagick). Not in the
> build depends though. The /usr/bin/convert program I had installed
> graphicsmagick-imagemagick-compat from was not the right one.

That patch has the problem (mentioned already on this list) that
_GNU_SOURCE is a user defined macro, and AC_USE_SYSTEM_EXTENSIONS will
unconditionally define it, so get_current_dir_name() would get used
regardless of availability. Instead I'd recommend something like the
following completely untested changes:

> --- a/configure.in	2012-06-20 17:53:17.000000000 +0200
> +++ b/configure.in	2012-06-20 18:01:24.000000000 +0200
> @@ -4,6 +4,7 @@
>  AC_CONFIG_HEADER([config.h])
>  AM_INIT_AUTOMAKE
>  AC_CANONICAL_HOST
> +AC_USE_SYSTEM_EXTENSIONS
AC_CHECK_FUNCS([get_current_dir_name])
>
>  #
>  #  1. Remember that version-info is current:revision:age, and age <= current.
> --- a/src/libpst.c	2011-12-25 00:19:11.000000000 +0100
> +++ b/src/libpst.c	2012-06-20 19:03:57.000000000 +0200

char *pst_getcwd(void) {
    char *cwd;

#ifdef HAVE_GET_CURRENT_DIR_NAME
    cwd = get_current_dir_name();
#else
    cwd = pst_malloc(PATH_MAX+1);
    getcwd(cwd, PATH_MAX+1);
#endif
    return cwd;
}

> @@ -361,19 +361,36 @@
>  
>      DEBUG_RET();
>  
       pf->cwd = pst_getcwd();
>      pf->fname = strdup(name);
>      return 0;
>  }
>  
>  
>  int  pst_reopen(pst_file *pf) {
       char *cwd;
       cwd = pst_getcwd();
       if (cwd == NULL)                       return -1;
       if (chdir(pf->cwd))                    goto err;
       if (!freopen(pf->fname, "rb", pf->fp)) goto err;
       if (chdir(cwd))                        goto err;
       free(cwd);
>      return 0;
err:
       free(cwd);
       return -1;
>  }

I don't generally like this type of error unwinding, but that depends
on the surrounding code, which I've not checked. Another option would
be to move the chdir/freopen/chdir operation into a static function,
call that from pst_reopen and free only once if that static funtion
fails.

regards,
guillem