Re: Comments on xerces-c patch please

[Guillem Jover <guillem@debian.org>]

Hi!

On Thu, 2011-08-04 at 00:14:05 +0200, Svante Signell wrote:
> One of the blahtexml build dependencies is xerces-c which is not
> available for the Hurd. The attached patch fixes the build. Can somebody
> check it the patch is OK before I submit a bug report? 

Well, you didn't give much time for feedback. Anyway here it goes:

> --- xerces-c-3.1.1/src/xercesc/util/FileManagers/PosixFileMgr.cpp.orig	2008-07-04 09:23:56.000000000 +0000
> +++ xerces-c-3.1.1/src/xercesc/util/FileManagers/PosixFileMgr.cpp	2011-08-03 21:58:23.000000000 +0000
> @@ -187,6 +187,17 @@
>      ArrayJanitor<char> janText(newSrc, manager);
>  
>      // Use a local buffer that is big enough for the largest legal path
> +#ifdef __GNU__
> +    char *absPath;
> +    XMLCh *ret;
> +    // get the absolute path
> +    absPath=realpath(newSrc, NULL);
> +    if (absPath == NULL)
> +   		ThrowXMLwithMemMgr(XMLPlatformUtilsException, XMLExcepts::File_CouldNotGetBasePathName, manager);
> +    ret = XMLString::transcode(absPath, manager);
> +    free (absPath);
> +    return ret; 
> +#else

Follow the existing coding style: add a blank line between the
variable definintions and the actual code block, add missing spaces
around =, remove trailing space after “return ret; ”.

Also given that realpath(path, NULL) is in POSIX.1-2008 and has been
supported for a long time on GNU/* and FreeBSD systems I'd change the
__GNU__ check to a configure check for the realpath functionality.

>      char absPath[PATH_MAX + 1];
>      
>      // get the absolute path
> @@ -194,20 +205,34 @@
>     		ThrowXMLwithMemMgr(XMLPlatformUtilsException, XMLExcepts::File_CouldNotGetBasePathName, manager);
>  
>      return XMLString::transcode(absPath, manager);
> +#endif
>  }
>  
>  
>  XMLCh*
>  PosixFileMgr::getCurrentDirectory(MemoryManager* const manager)
>  {
> +#ifdef __GNU__
> +    char *dirBuf=NULL;
> +    XMLCh *ret;
> +    size_t dummy = 0;
> +    char *curDir = getcwd(dirBuf, dummy);
> +    if (curDir == NULL)
> +        ThrowXMLwithMemMgr(XMLPlatformUtilsException,
> +                 XMLExcepts::File_CouldNotGetBasePathName, manager);
> +
> +    ret = XMLString::transcode(curDir, manager);
> +    free (curDir);
> +    return ret;
> +#else

There's no need for the dummy or dirBuf variables. The same style
comments as the above. And getcwd(NULL, 0) has also been supported on
FreeBSD and GNU/* for a long time, it would be better to check for its
availabilty at configure time and use that whenever possible.

>      char dirBuf[PATH_MAX + 2];
>      char *curDir = getcwd(&dirBuf[0], PATH_MAX + 1);
> -

Do not change unrelated spaces and blank lines for other code blocks.

>      if (!curDir)
>          ThrowXMLwithMemMgr(XMLPlatformUtilsException,
>                   XMLExcepts::File_CouldNotGetBasePathName, manager);
>  
>      return XMLString::transcode(curDir, manager);
> +#endif
>  }

regards,
guillem