On Tue, 2011-04-12 at 20:56 +0200, Samuel Thibault wrote: > Svante Signell, le Tue 12 Apr 2011 19:27:10 +0200, a écrit : > > return -1 ; > > } > > /* If we reach here, "filename" is not a symbolic link */ > > And thus we didn't call realpath at all. I believe the free below > shouldn't be done, it's probably only by chance that it doesn't segfault > in your test (by being NULL by chance).
Yes, we did not call realpath but the variable is still declared but not malloced. Does that mean we shouldn't free() it? > > +#ifdef __GNU__ > > + free (realfilepath); > > +#endif > > return openandconvert( filename ); > > } OK, thanks for your comments. The biggest problem with malloced storage, is where to put the free() statements. To do that you need to follow the program logic and find out every statement with a possible exit. This can be very difficult sometimes, at least I think so. Final version??
diff -ur ./tofrodos-1.7.8.debian.1/src/config.h tofrodos-1.7.8.debian.1.new/src/config.h --- tofrodos-1.7.8.debian.1/src/config.h 2011-04-12 19:02:08.000000000 +0200 +++ tofrodos-1.7.8.debian.1.new/src/config.h 2011-04-12 19:09:44.000000000 +0200 @@ -102,7 +102,7 @@ #endif #endif -#if defined(__FreeBSD__) || defined(__OpenBSD__) /* seems to work like Linux... */ +#if defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__GNU__) /* seems to work like Linux... */ #if !defined(LINUX) #define LINUX #endif 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 @@ -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 /* 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 if (verbose) { emsg( VERBOSE_SYMLINKSRC, filename, realfilepath ); } +#ifndef __GNU__ return openandconvert( realfilepath ); +#else + len = openandconvert( realfilepath ); + free (realfilepath); + return len; +#endif } emsg( EMSG_SYMLINK, filename ); +#ifdef __GNU__ + free (realfilepath); +#endif return -1 ; } /* If we reach here, "filename" is not a symbolic link */