From: Theodore Ts'o Date: Fri, 3 Jan 2014 05:26:43 +0000 (-0500) Subject: subst: clean up various coverity nits X-Git-Tag: v1.42.10~145 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2873927d15ffb9ee9ed0e2700791a0e519c715aa;p=thirdparty%2Fe2fsprogs.git subst: clean up various coverity nits Add appropriate error checking for all error returns, and only open each file that we need to manipulate once, to avoid potential time-of-check/time-of-use races. (Not that this is likely for this program, but the result is much more clean.) We also preserve the atime in the case where the file has not changed. Addresses-Coverty-Id: #709537 Addresses-Coverty-Id: #1049150 Addresses-Coverty-Id: #1049151 Signed-off-by: "Theodore Ts'o" --- diff --git a/configure b/configure index 31ec8d395..6f8f1ab73 100755 --- a/configure +++ b/configure @@ -10446,6 +10446,16 @@ if test "x$ac_cv_member_struct_dirent_d_reclen" = xyes; then : $as_echo "#define HAVE_RECLEN_DIRENT 1" >>confdefs.h +fi + +ac_fn_c_check_member "$LINENO" "struct stat" "st_atim" "ac_cv_member_struct_stat_st_atim" "$ac_includes_default" +if test "x$ac_cv_member_struct_stat_st_atim" = xyes; then : + +cat >>confdefs.h <<_ACEOF +#define HAVE_STRUCT_STAT_ST_ATIM 1 +_ACEOF + + fi ac_fn_c_check_type "$LINENO" "ssize_t" "ac_cv_type_ssize_t" "#include @@ -11041,7 +11051,7 @@ if test "$ac_res" != no; then : fi fi -for ac_func in __secure_getenv backtrace blkid_probe_get_topology chflags fallocate fallocate64 fchown fdatasync fstat64 ftruncate64 getdtablesize getmntinfo getpwuid_r getrlimit getrusage jrand48 llseek lseek64 mallinfo mbstowcs memalign mmap msync nanosleep open64 pathconf posix_fadvise posix_memalign prctl secure_getenv setmntent setresgid setresuid srandom strcasecmp strdup strnlen strptime strtoull sync_file_range sysconf usleep utime valloc +for ac_func in __secure_getenv backtrace blkid_probe_get_topology chflags fallocate fallocate64 fchown fdatasync fstat64 ftruncate64 futimes getdtablesize getmntinfo getpwuid_r getrlimit getrusage jrand48 llseek lseek64 mallinfo mbstowcs memalign mmap msync nanosleep open64 pathconf posix_fadvise posix_memalign prctl secure_getenv setmntent setresgid setresuid srandom strcasecmp strdup strnlen strptime strtoull sync_file_range sysconf usleep utime valloc do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" @@ -11427,7 +11437,7 @@ if test "$USE_INCLUDED_LIBINTL" = "yes" ; then fi if test $cross_compiling = no; then - BUILD_CFLAGS="$CFLAGS $CPPFLAGS" + BUILD_CFLAGS="$CFLAGS $CPPFLAGS $INCLUDES -DHAVE_CONFIG_H" BUILD_LDFLAGS="$LDFLAGS" else BUILD_CFLAGS= diff --git a/configure.in b/configure.in index 6f39cc983..b70a3f9a9 100644 --- a/configure.in +++ b/configure.in @@ -909,6 +909,7 @@ dnl is not decleared. AC_CHECK_MEMBER(struct dirent.d_reclen,[AC_DEFINE(HAVE_RECLEN_DIRENT, 1, [Define to 1 if dirent has d_reclen])],, [#include ]) +AC_CHECK_MEMBERS([struct stat.st_atim]) dnl Check to see if ssize_t was declared AC_CHECK_TYPE(ssize_t,[AC_DEFINE(HAVE_TYPE_SSIZE_T, 1, [Define to 1 if ssize_t declared])],, @@ -1031,6 +1032,7 @@ AC_CHECK_FUNCS(m4_flatten([ fdatasync fstat64 ftruncate64 + futimes getdtablesize getmntinfo getpwuid_r @@ -1280,7 +1282,7 @@ dnl dnl Build CFLAGS dnl if test $cross_compiling = no; then - BUILD_CFLAGS="$CFLAGS $CPPFLAGS" + BUILD_CFLAGS="$CFLAGS $CPPFLAGS $INCLUDES -DHAVE_CONFIG_H" BUILD_LDFLAGS="$LDFLAGS" else BUILD_CFLAGS= diff --git a/lib/config.h.in b/lib/config.h.in index 284eb3334..ef2e6647d 100644 --- a/lib/config.h.in +++ b/lib/config.h.in @@ -121,6 +121,9 @@ /* Define to 1 if you have the `ftruncate64' function. */ #undef HAVE_FTRUNCATE64 +/* Define to 1 if you have the `futimes' function. */ +#undef HAVE_FUTIMES + /* Define to 1 if you have the `fwprintf' function. */ #undef HAVE_FWPRINTF @@ -384,6 +387,9 @@ /* Define to 1 if you have the `strtoull' function. */ #undef HAVE_STRTOULL +/* Define to 1 if `st_atim' is a member of `struct stat'. */ +#undef HAVE_STRUCT_STAT_ST_ATIM + /* Define to 1 if you have the `sync_file_range' function. */ #undef HAVE_SYNC_FILE_RANGE diff --git a/util/Makefile.in b/util/Makefile.in index 76c3f88ac..2a2b21c29 100644 --- a/util/Makefile.in +++ b/util/Makefile.in @@ -22,6 +22,12 @@ PROGS= subst symlinks all:: $(PROGS) gen-tarball +dirpaths.h: + $(E) " CREATE dirpaths.h" + $(Q) echo "/* fake dirpaths.h for config.h */" > dirpaths.h + +subst.o: dirpaths.h + subst: subst.o $(E) " LD $@" $(Q) $(BUILD_CC) $(BUILD_LDFLAGS) -o subst subst.o @@ -46,7 +52,7 @@ tarballs: gen-tarball clean: $(RM) -f $(PROGS) \#* *.s *.o *.a *~ core *.tar.gz gen-tarball \ - copy-sparse + copy-sparse dirpaths.h mostlyclean: clean diff --git a/util/subst.c b/util/subst.c index 20dd6f21a..624483122 100644 --- a/util/subst.c +++ b/util/subst.c @@ -5,6 +5,9 @@ * */ +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif #include #include #include @@ -13,6 +16,7 @@ #include #include #include +#include #include #include @@ -264,21 +268,11 @@ static void parse_config_file(FILE *f) /* * Return 0 if the files are different, 1 if the files are the same. */ -static int compare_file(const char *outfn, const char *newfn) +static int compare_file(FILE *old_f, FILE *new_f) { - FILE *old_f, *new_f; char oldbuf[2048], newbuf[2048], *oldcp, *newcp; int retval; - old_f = fopen(outfn, "r"); - if (!old_f) - return 0; - new_f = fopen(newfn, "r"); - if (!new_f) { - fclose(old_f); - return 0; - } - while (1) { oldcp = fgets(oldbuf, sizeof(oldbuf), old_f); newcp = fgets(newbuf, sizeof(newbuf), new_f); @@ -291,8 +285,6 @@ static int compare_file(const char *outfn, const char *newfn) break; } } - fclose(old_f); - fclose(new_f); return retval; } @@ -302,12 +294,14 @@ int main(int argc, char **argv) { char line[2048]; int c; - FILE *in, *out; + int fd; + FILE *in, *out, *old = NULL; char *outfn = NULL, *newfn = NULL; int verbose = 0; int adjust_timestamp = 0; + int got_atime = 0; struct stat stbuf; - struct utimbuf ut; + struct timeval tv[2]; while ((c = getopt (argc, argv, "f:tv")) != EOF) { switch (c) { @@ -351,11 +345,34 @@ int main(int argc, char **argv) } strcpy(newfn, outfn); strcat(newfn, ".new"); - out = fopen(newfn, "w"); - if (!out) { + fd = open(newfn, O_CREAT|O_TRUNC|O_RDWR, 0444); + if (fd < 0) { perror(newfn); exit(1); } + out = fdopen(fd, "w+"); + if (!out) { + perror("fdopen"); + exit(1); + } + + fd = open(outfn, O_RDONLY); + if (fd > 0) { + /* save the original atime, if possible */ + if (fstat(fd, &stbuf) == 0) { +#if HAVE_STRUCT_STAT_ST_ATIM + tv[0].tv_sec = stbuf.st_atim.tv_sec; + tv[0].tv_usec = stbuf.st_atim.tv_nsec / 1000; +#else + tv[0].tv_sec = stbuf.st_atime; + tv[0].tv_usec = 0; +#endif + got_atime = 1; + } + old = fdopen(fd, "r"); + if (!old) + close(fd); + } } else { out = stdout; outfn = 0; @@ -368,32 +385,49 @@ int main(int argc, char **argv) fputs(line, out); } fclose(in); - fclose(out); if (outfn) { - struct stat st; - if (compare_file(outfn, newfn)) { + fflush(out); + rewind(out); + if (old && compare_file(old, out)) { if (verbose) printf("No change, keeping %s.\n", outfn); if (adjust_timestamp) { - if (stat(outfn, &stbuf) == 0) { - if (verbose) - printf("Updating modtime for %s\n", outfn); - ut.actime = stbuf.st_atime; - ut.modtime = time(0); - if (utime(outfn, &ut) < 0) - perror("utime"); + if (verbose) + printf("Updating modtime for %s\n", outfn); + if (gettimeofday(&tv[1], NULL) < 0) { + perror("gettimeofday"); + exit(1); } + if (got_atime == 0) + tv[0] = tv[1]; + else if (verbose) + printf("Using original atime\n"); +#ifdef HAVE_FUTIMES + if (futimes(fileno(old), tv) < 0) + perror("futimes"); +#else + if (utimes(outfn, tv) < 0) + perror("utimes"); +#endif } - unlink(newfn); + fclose(out); + if (unlink(newfn) < 0) + perror("unlink"); } else { if (verbose) printf("Creating or replacing %s.\n", outfn); - rename(newfn, outfn); + fclose(out); + if (old) + fclose(old); + old = NULL; + if (rename(newfn, outfn) < 0) { + perror("rename"); + exit(1); + } } - /* set read-only to alert user it is a generated file */ - if (stat(outfn, &st) == 0) - chmod(outfn, st.st_mode & ~0222); } + if (old) + fclose(old); return (0); }