From 8f9b843990a15c5aaab0e504978a13addccc6564 Mon Sep 17 00:00:00 2001 From: Christophe CURIS Date: Sun, 18 May 2014 00:56:40 +0200 Subject: [PATCH] WUtil: fixed possible problem in wcopy_file (Coverity #50141) As pointed by Coverity, the macro RETRY does not behave as expected, as it assumes that errno is cleared on successful 'fopen' call which is not the case. This patch removes the uses of the macro RETRY: - fopen: with the appropriate check - fread/fwrite: nothing because they do not set errno - fclose: nothing because retrying is not recommended and took the opportunity to add a little bit more information in the error messages. Signed-off-by: Christophe CURIS --- WINGs/findfile.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/WINGs/findfile.c b/WINGs/findfile.c index 048731ec..ea7980b8 100644 --- a/WINGs/findfile.c +++ b/WINGs/findfile.c @@ -36,7 +36,6 @@ #define PATH_MAX 1024 #endif -#define RETRY( x ) do { x; } while (errno == EINTR); char *wgethomedir() { @@ -431,27 +430,31 @@ int wcopy_file(const char *dir, const char *src_file, const char *dest_file) if (stat(src_file, &st) != 0 || !S_ISREG(st.st_mode)) return -1; - RETRY( src = fopen(src_file, "rb") ) + do { + src = fopen(src_file, "rb"); + } while ((src == NULL) && (errno == EINTR)); if (src == NULL) { - werror(_("Could not open %s"), src_file); + werror(_("Could not open input file \"%s\""), src_file); return -1; } dstpath = wstrconcat(dir, dest_file); - RETRY( dst = fopen(dstpath, "wb") ) + do { + dst = fopen(dstpath, "wb"); + } while ((dst == NULL) && (errno == EINTR)); if (dst == NULL) { - werror(_("Could not create %s"), dstpath); + werror(_("Could not create target file \"%s\""), dstpath); wfree(dstpath); - RETRY( fclose(src) ) + fclose(src); return -1; } do { - RETRY( nread = fread(buf, 1, sizeof(buf), src) ) + nread = fread(buf, 1, sizeof(buf), src); if (ferror(src)) break; - RETRY( nwritten = fwrite(buf, 1, nread, dst) ) + nwritten = fwrite(buf, 1, nread, dst); if (ferror(dst) || feof(src) || nread != nwritten) break; @@ -460,10 +463,11 @@ int wcopy_file(const char *dir, const char *src_file, const char *dest_file) if (ferror(src) || ferror(dst)) unlink(dstpath); - RETRY( fclose(src) ) + fclose(src); fchmod(fileno(dst), st.st_mode); fsync(fileno(dst)); - RETRY( fclose(dst) ) + if (fclose(dst)) + wwarning("error occured during fclose(\"%s\")", dstpath); wfree(dstpath); return 0;