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" <tytso@mit.edu>

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 <sys/types.h>
@@ -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=

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 <dirent.h>])
+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=

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
 

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
 

util/subst.c

@@ -5,6 +5,9 @@
  *
  */
 
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
 #include <stdio.h>
 #include <errno.h>
 #include <stdlib.h>
@@ -13,6 +16,7 @@
 #include <ctype.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <fcntl.h>
 #include <time.h>
 #include <utime.h>
 
@@ -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);
 }