From 71aa4f2884ba8c5e6f284249951b07b5f5092d9d Mon Sep 17 00:00:00 2001 From: Tamas TEVESZ Date: Fri, 26 Mar 2010 20:25:27 +0100 Subject: [PATCH] Switch file ops to stdio - Does away with the O_BINARY abomination - as a byproduct, plugs an fd leak in wcolorpanel.c:fetchFile() - sprinkle some fsync()s to files that have been written to (this needs to be done everywhere) + fix brown paper bag thinko in configure.ac --- WINGs/proplist.c | 1 + WINGs/wcolorpanel.c | 60 ++++++++++++++++++++--------------- WPrefs.app/Appearance.c | 1 + WPrefs.app/MouseSettings.c | 5 ++- configure.ac | 3 +- src/osdep_linux.c | 12 ++++--- util/getstyle.c | 65 ++++++++++++++++++++++++++------------ wrlib/load.c | 23 ++++++++------ 8 files changed, 108 insertions(+), 62 deletions(-) diff --git a/WINGs/proplist.c b/WINGs/proplist.c index f4d9c765..476fa659 100644 --- a/WINGs/proplist.c +++ b/WINGs/proplist.c @@ -1603,6 +1603,7 @@ Bool WMWritePropListToFile(WMPropList * plist, char *path) wfree(desc); + (void)fsync(fileno(theFile)); if (fclose(theFile) != 0) { wsyserror(_("fclose (%s) failed"), thePath); goto failure; diff --git a/WINGs/wcolorpanel.c b/WINGs/wcolorpanel.c index a9c4def0..6a6ebd05 100644 --- a/WINGs/wcolorpanel.c +++ b/WINGs/wcolorpanel.c @@ -26,6 +26,8 @@ #include "wconfig.h" #include "WINGsP.h" #include "rgb.h" + +#include #include #include #include @@ -33,7 +35,10 @@ #include #include #include -#include + +#define RETRY( x ) do { \ + x; \ + } while (errno == EINTR); /* BUG There's something fishy with shaped windows */ /* Whithout shape extension the magnified image is completely broken -Dan */ @@ -270,11 +275,6 @@ enum { #define M_PI 3.14159265358979323846 #endif -/* Silly hack for Windows systems with cygwin */ -#ifndef O_BINARY -# define O_BINARY 0 -#endif - static int fetchFile(char *toPath, char *imageSrcFile, char *imageDestFileName); char *generateNewFilename(char *curName); void convertCPColor(CPColor * color); @@ -3343,35 +3343,45 @@ static void hsbInit(W_ColorPanel * panel) static int fetchFile(char *toPath, char *srcFile, char *destFile) { - int src, dest; - int n; - char *tmp; + FILE *src, *dst; + size_t nread, nwritten; + char *dstpath; char buf[BUFSIZE]; - if ((src = open(srcFile, O_RDONLY | O_BINARY)) == 0) { + RETRY( src = fopen(srcFile, "rb") ) + if (src == NULL) { wsyserror(_("Could not open %s"), srcFile); return -1; } - tmp = wstrconcat(toPath, destFile); - if ((dest = open(tmp, O_RDWR | O_CREAT | O_BINARY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH)) - == 0) { - wsyserror(_("Could not create %s"), tmp); - wfree(tmp); + dstpath = wstrconcat(toPath, destFile); + RETRY( dst = fopen(dstpath, "wb") ) + if (dst == NULL) { + wsyserror(_("Could not create %s"), dstpath); + wfree(dstpath); + RETRY( fclose(src) ) return -1; } - wfree(tmp); - /* Copy the file */ - while ((n = read(src, buf, BUFSIZE)) > 0) { - if (write(dest, buf, n) != n) { - wsyserror(_("Write error on file %s"), destFile); - return -1; - } - } + do { + RETRY( nread = fread(buf, 1, sizeof(buf), src) ) + if (ferror(src)) + break; - close(src); - close(dest); + RETRY( nwritten = fwrite(buf, 1, nread, dst) ) + if (ferror(dst) || feof(src)) + break; + + } while (1); + + if (ferror(src) || ferror(dst)) + unlink(dstpath); + + RETRY( fclose(src) ) + fsync(fileno(dst)); + RETRY( fclose(dst) ) + + wfree(dstpath); return 0; } diff --git a/WPrefs.app/Appearance.c b/WPrefs.app/Appearance.c index f098d971..106a306a 100644 --- a/WPrefs.app/Appearance.c +++ b/WPrefs.app/Appearance.c @@ -402,6 +402,7 @@ static void dumpRImage(char *path, RImage * image) fwrite(image->data, 1, image->width * image->height * channels, f); + fsync(fileno(f)); if (fclose(f) < 0) { wsyserror(path); } diff --git a/WPrefs.app/MouseSettings.c b/WPrefs.app/MouseSettings.c index 86b1e6f2..ab240617 100644 --- a/WPrefs.app/MouseSettings.c +++ b/WPrefs.app/MouseSettings.c @@ -699,6 +699,7 @@ static void storeCommandInScript(char *cmd, char *line) fputs(line, fo); fputs("\n", fo); } + fsync(fileno(fo)); fclose(fo); if (rename(tmppath, path) != 0) { @@ -711,8 +712,10 @@ static void storeCommandInScript(char *cmd, char *line) end: wfree(path); - if (f) + if (f) { + fsync(fileno(f)); /* this may be rw */ fclose(f); + } } static void storeData(_Panel * panel) diff --git a/configure.ac b/configure.ac index a9619bcd..3133f00c 100644 --- a/configure.ac +++ b/configure.ac @@ -41,8 +41,7 @@ dnl Platform-specific Makefile setup dnl ================================ case "${host}" in - *-*-cygwin*) - *-*-linux*) + *-*-linux*|*-*-cygwin*) WM_OSDEP="linux" ;; *-*-freebsd*) diff --git a/src/osdep_linux.c b/src/osdep_linux.c index ed4953f9..fd97f59b 100644 --- a/src/osdep_linux.c +++ b/src/osdep_linux.c @@ -13,6 +13,10 @@ #include "wconfig.h" +#define RETRY( x ) do { \ + x; \ + } while (errno == EINTR); + /* * copy argc and argv for an existing process identified by `pid' * into suitable storage given in ***argv and *argc. @@ -34,6 +38,8 @@ Bool GetCommandForPid(int pid, char ***argv, int *argc) /* cmdline is a flattened series of null-terminated strings */ snprintf(buf, sizeof(buf), "/proc/%d/cmdline", pid); while (1) { + /* not switching this to stdio yet, as this does not need + * to be portable, and i'm lazy */ if ((fd = open(buf, O_RDONLY)) != -1) break; if (errno == EINTR) @@ -46,12 +52,10 @@ Bool GetCommandForPid(int pid, char ***argv, int *argc) break; if (errno == EINTR) continue; + RETRY( close(fd) ) return False; } - - do { - close(fd); - } while (errno == EINTR); + RETRY( close(fd) ) /* count args */ for (i = 0; i < count; i++) diff --git a/util/getstyle.c b/util/getstyle.c index 69b37bbb..78f9efcd 100644 --- a/util/getstyle.c +++ b/util/getstyle.c @@ -25,6 +25,7 @@ #include #include +#include #include #include #include @@ -37,6 +38,10 @@ #include #include +#define RETRY( x ) do { \ + x; \ + } while (errno == EINTR); + #ifndef PATH_MAX #define PATH_MAX 1024 #endif @@ -161,17 +166,21 @@ static Bool isFontOption(char *option) return False; } +/* + * copy a file specified by `file' into `directory'. name stays. + */ /* * it is more or less assumed that this function will only * copy reasonably-sized files */ +/* XXX: is almost like WINGs/wcolodpanel.c:fetchFile() */ void copyFile(char *dir, char *file) { - int from_fd, to_fd; - size_t block, len; + FILE *src, *dst; + size_t nread, nwritten, len; char buf[4096]; struct stat st; - char *dst; + char *dstpath; /* only to a directory */ if (stat(dir, &st) != 0 || !S_ISDIR(st.st_mode)) @@ -181,28 +190,44 @@ void copyFile(char *dir, char *file) return; len = strlen(dir) + 1 /* / */ + strlen(file) + 1 /* '\0' */; - dst = wmalloc(len); - snprintf(dst, len, "%s/%s", dir, basename(file)); + dstpath = wmalloc(len); + snprintf(dstpath, len, "%s/%s", dir, basename(file)); buf[len] = '\0'; - if ((to_fd = open(dst, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)) == -1) { - wfree(dst); - return; - } - wfree(dst); - if ((from_fd = open(file, O_RDONLY)) == -1) { - (void)close(to_fd); - return; + RETRY( dst = fopen(dstpath, "wb") ) + if (dst == NULL) { + wsyserror(_("Could not create %s"), dstpath); + goto err; } - /* XXX: signal handling */ - while ((block = read(from_fd, &buf, sizeof(buf))) > 0) - write(to_fd, &buf, block); + RETRY( src = fopen(file, "rb") ) + if (src == NULL) { + wsyserror(_("Could not open %s"), file); + goto err; + } - (void)fsync(to_fd); - (void)fchmod(to_fd, st.st_mode); - (void)close(to_fd); - (void)close(from_fd); + do { + RETRY( nread = fread(buf, 1, sizeof(buf), src) ) + if (ferror(src)) + break; + + RETRY( nwritten = fwrite(buf, 1, nread, dst) ) + if (ferror(dst) || feof(src)) + break; + + } while (1); + + if (ferror(src) || ferror(dst)) + unlink(dstpath); + + fchmod(fileno(dst), st.st_mode); + fsync(fileno(dst)); + RETRY( fclose(dst) ) + +err: + RETRY( fclose(src) ) + wfree(dstpath); + return; } void findCopyFile(char *dir, char *file) diff --git a/wrlib/load.c b/wrlib/load.c index bfd46b94..2d577eee 100644 --- a/wrlib/load.c +++ b/wrlib/load.c @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -37,10 +38,9 @@ #include "wraster.h" -/* Silly hack for Windows systems with cygwin */ -#ifndef O_BINARY -# define O_BINARY 0 -#endif +#define RETRY( x ) do { \ + x; \ + } while (errno == EINTR); typedef struct RCachedImage { RImage *image; @@ -299,22 +299,25 @@ char *RGetImageFileFormat(char *file) static int identFile(char *path) { - int fd; + FILE *file; unsigned char buffer[32]; + size_t nread; assert(path != NULL); - fd = open(path, O_RDONLY | O_BINARY); - if (fd < 0) { + RETRY( file = fopen(path, "rb") ) + if (file == NULL) { RErrorCode = RERR_OPEN; return IM_ERROR; } - if (read(fd, buffer, 32) < 1) { - close(fd); + + RETRY( nread = fread(buffer, 1, sizeof(buffer), file) ) + if (nread < sizeof(buffer) || ferror(file)) { + RETRY( fclose(file) ) RErrorCode = RERR_READ; return IM_ERROR; } - close(fd); + RETRY( fclose(file) ) /* check for XPM */ if (strncmp((char *)buffer, "/* XPM */", 9) == 0)