Re: Bug#622365: Fix FTBFS of tofrodos for GNU/Hurd
Hi!
On Tue, 2011-04-12 at 21:46:09 +0200, Svante Signell wrote:
> Final version??
> diff -ur tofrodos-1.7.8.debian.1/src/tofrodos.c tofrodos-1.7.8.debian.1.new/src/tofrodos.c
> --- tofrodos-1.7.8.debian.1/src/tofrodos.c 2011-04-12 18:21:50.000000000 +0200
> +++ tofrodos-1.7.8.debian.1/src/tofrodos.c.new 2011-04-12 21:24:27.000000000 +0200
> @@ -14,7 +14,9 @@
> #include <stdlib.h> /* EXIT_SUCCESS, mkstemp() in some systems */
> #include <string.h> /* strrchr(), strlen(), strcpy(), strcat() */
> #include <sys/stat.h> /* stat() */
> -
> +#ifdef __GNU__
> +#include <limits.h>
> +#endif
> #if defined(UNIX)
> #if defined(MAXPATHLEN_HEADER)
> #include MAXPATHLEN_HEADER
Why are you including limits.h, it does not seem to be needed.
> @@ -443,8 +445,12 @@
> static int openandconvert_preamble ( char * filename )
> {
> struct stat statbuf ;
> - char realfilepath[MAXPATHLEN+1] ;
> int len ;
> +#ifndef __GNU__
> + char realfilepath[MAXPATHLEN+1] ;
> +#else
> + char *realfilepath ;
> +#endif
The dynamic memory code should work everywhere, I don't see the need
to complicate things by having more system dependant code paths.
> /* get the file information */
> if (lstat( filename, &statbuf )) {
> @@ -461,16 +467,30 @@
> /* eg, #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK) */
> /* or something like that. */
>
> +#ifndef __GNU__
> if ((len = readlink( filename, realfilepath, (sizeof(realfilepath) - 1) )) != -1) {
> /* got to null terminate the string - there is always space because */
> /* we passed readlink() the size of the buffer less 1. */
> realfilepath[len] = '\0' ;
> +#else
> + realfilepath = realpath (filename, NULL);
> + if ((realfilepath) != NULL) {
> +#endif
So, while doing realpath() would seem to be the correct thing to do
everywhere, as the initial symlink might point to another symlink,
this changes behaviour only for Hurd, which I think it's a bad idea.
But using realpath(foo, NULL) is not really portable and would need to
be checked at build time if it was to be used everywhere.
So what about the attached patch instead (which I had around from some
time ago when I took a look at this, although forgot it in my kvm :/,
beware only build tested).
It also changes the kFreeBSD patch because usually __GLIBC__ is not
predefined and it seems confusing to have it in there. (Ideally the
build system would switch to detect features at build time, instead of
hard-coding system support.)
thanks,
guillem
diff --git a/debian/patches/FTBFS_kfreebsd.diff b/debian/patches/FTBFS_kfreebsd.diff
deleted file mode 100644
index 0cd31b6..0000000
--- a/debian/patches/FTBFS_kfreebsd.diff
+++ /dev/null
@@ -1,16 +0,0 @@
-## 02_FTBFS_kfreebsd.dpatch by Florian Ernst <florian@debian.org>
-## All lines beginning with `## DP:' are a description of the patch.
-## DP: Fix FTBFS on GNU/kFreeBSD, patch by Robert Millan, see bug#351417
-
-diff -urNad tofrodos-1.7.6~/src/config.h tofrodos-1.7.6/src/config.h
---- tofrodos-1.7.6~/src/config.h 2005-03-15 16:01:02.000000000 +0100
-+++ tofrodos-1.7.6/src/config.h 2006-02-07 12:01:04.000000000 +0100
-@@ -95,7 +95,7 @@
- #endif
-
- /* define the systems */
--#if defined(__linux__) /* (predefined) */
-+#if defined(__linux__) || defined(__GLIBC__) /* (predefined) */
- #if !defined(LINUX)
- #define LINUX
- #endif
diff --git a/debian/patches/FTBFS_non-linux.diff b/debian/patches/FTBFS_non-linux.diff
new file mode 100644
index 0000000..ef3db9a
--- /dev/null
+++ b/debian/patches/FTBFS_non-linux.diff
@@ -0,0 +1,59 @@
+## 02_FTBFS_non-linux.dpatch by Guillem Jover <guillem@debian.org>
+## All lines beginning with `## DP:' are a description of the patch.
+## DP: Fix FTBFS on non Linux systems
+
+---
+ src/config.h | 4 +++-
+ src/tofrodos.c | 13 ++++++++++---
+ 2 files changed, 13 insertions(+), 4 deletions(-)
+
+--- a/src/config.h
++++ b/src/config.h
+@@ -102,7 +102,9 @@ extern "C" {
+ #endif
+ #endif
+
+-#if defined(__FreeBSD__) || defined(__OpenBSD__) /* seems to work like Linux... */
++#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || \
++ defined(__OpenBSD__) || defined(__NetBSD__) || \
++ defined(__GNU__) /* seems to work like Linux... */
+ #if !defined(LINUX)
+ #define LINUX
+ #endif
+--- a/src/tofrodos.c
++++ b/src/tofrodos.c
+@@ -443,7 +443,7 @@ static int convert ( FILE * infp, FILE *
+ static int openandconvert_preamble ( char * filename )
+ {
+ struct stat statbuf ;
+- char realfilepath[MAXPATHLEN+1] ;
++ char *realfilepath ;
+ int len ;
+
+ /* get the file information */
+@@ -461,16 +461,23 @@ static int openandconvert_preamble ( cha
+ /* eg, #define S_ISLNK(x) (((x) & S_IFMT) == S_IFLNK) */
+ /* or something like that. */
+
+- if ((len = readlink( filename, realfilepath, (sizeof(realfilepath) - 1) )) != -1) {
++ realfilepath = xmalloc( statbuf.st_size + 1 );
++
++ if ((len = readlink( filename, realfilepath, statbuf.st_size )) != -1) {
++ int ret ;
++
+ /* got to null terminate the string - there is always space because */
+ /* we passed readlink() the size of the buffer less 1. */
+ realfilepath[len] = '\0' ;
+ if (verbose) {
+ emsg( VERBOSE_SYMLINKSRC, filename, realfilepath );
+ }
+- return openandconvert( realfilepath );
++ ret = openandconvert( realfilepath );
++ free( realfilepath );
++ return ret;
+ }
+ emsg( EMSG_SYMLINK, filename );
++ free( realfilepath );
+ return -1 ;
+ }
+ /* If we reach here, "filename" is not a symbolic link */
diff --git a/debian/patches/series b/debian/patches/series
index 7252415..5b0a355 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1 +1 @@
-FTBFS_kfreebsd.diff
+FTBFS_non-linux.diff
Reply to: