Addresses-Coverity-ID: #1147783
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
misc/e2image.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/misc/e2image.c b/misc/e2image.c
index ab6a4fa..253fad1 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -174,7 +174,7 @@ static void generic_write(int fd, void *buf, int blocksize, blk64_t block)
printf(_("Writing block %llu\n"), (unsigned long long) block);
if (fd != 1)
seek_relative(fd, blocksize);
- return;
+ goto free_and_return;
}
count = write(fd, buf, blocksize);
if (count != blocksize) {
@@ -191,6 +191,7 @@ static void generic_write(int fd, void *buf, int blocksize, blk64_t block)
exit(1);
}
+free_and_return:
if (free_buf)
ext2fs_free_mem(&buf);
}
--
1.8.5.rc3.362.gdf10213
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" <[email protected]>
---
configure | 12 +++++++-
configure.in | 3 +-
lib/config.h.in | 6 ++++
util/subst.c | 93 +++++++++++++++++++++++++++++++++++++--------------------
4 files changed, 80 insertions(+), 34 deletions(-)
diff --git a/configure b/configure
index 95ce324..6661265 100755
--- a/configure
+++ b/configure
@@ -10448,6 +10448,16 @@ $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>
"
if test "x$ac_cv_type_ssize_t" = xyes; then :
@@ -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 2b0087e..d6a3796 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 <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])],,
@@ -1281,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 284eb33..ada69ed 100644
--- a/lib/config.h.in
+++ b/lib/config.h.in
@@ -284,6 +284,9 @@
/* Define to 1 if you have the `posix_fadvise' function. */
#undef HAVE_POSIX_FADVISE
+/* Define to 1 if you have the `posix_fadvise64' function. */
+#undef HAVE_POSIX_FADVISE64
+
/* Define to 1 if you have the `posix_memalign' function. */
#undef HAVE_POSIX_MEMALIGN
@@ -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/subst.c b/util/subst.c
index 20dd6f2..f2e2424 100644
--- a/util/subst.c
+++ b/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,44 @@ 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");
+ if (futimes(fileno(old), tv) < 0)
+ perror("futimes");
}
- 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);
}
--
1.8.5.rc3.362.gdf10213
Dividing a floating point number by zero is undefined in C. It
happens to work with gcc/glibc, but it's not something that's
guaranteed.
Addresses-Coverity-ID: #1147781
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
misc/e2image.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/misc/e2image.c b/misc/e2image.c
index 253fad1..ac62ffe 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -704,14 +704,15 @@ more_blocks:
if (show_progress) {
time_t duration = time(NULL) - start_time;
char buff[30];
- while (bscount--)
- fputc('\b', stderr);
+ fputc('\r', stderr);
strftime(buff, 30, "%T", gmtime(&duration));
- fprintf(stderr, _("\b\b\b\b\b\b\b\bCopied %llu / %llu "
- "blocks (%d%%) in %s at %.2f MB/s \n"),
- total_written, meta_blocks_count,
- calc_percent(total_written, meta_blocks_count), buff,
- calc_rate(total_written, fs->blocksize, duration));
+ fprintf(stderr, _("Copied %llu / %llu blocks (%d%%) in %s "),
+ total_written, meta_blocks_count,
+ calc_percent(total_written, meta_blocks_count), buff);
+ if (duration)
+ fprintf(stderr, _("at %.2f MB/s"),
+ calc_rate(total_written, fs->blocksize, duration));
+ fputs(" \n", stderr);
}
#ifdef HAVE_FTRUNCATE64
if (sparse) {
--
1.8.5.rc3.362.gdf10213
Addresses-Coverity-Bug: #709510
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
lib/blkid/probe.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/blkid/probe.c b/lib/blkid/probe.c
index bd31fe0..6f74bd4 100644
--- a/lib/blkid/probe.c
+++ b/lib/blkid/probe.c
@@ -1003,7 +1003,7 @@ static int probe_udf(struct blkid_probe *probe,
(block sizes larger than 2K will be null padded) */
for (bs = 1; bs < 16; bs++) {
isosb = (struct iso_volume_descriptor *)
- get_buffer(probe, bs*2048+32768, sizeof(isosb));
+ get_buffer(probe, bs*2048+32768, sizeof(*isosb));
if (!isosb)
return 1;
if (isosb->vd_id[0])
@@ -1015,7 +1015,7 @@ static int probe_udf(struct blkid_probe *probe,
if (j > 1) {
isosb = (struct iso_volume_descriptor *)
get_buffer(probe, j*bs*2048+32768,
- sizeof(isosb));
+ sizeof(*isosb));
if (!isosb)
return 1;
}
@@ -1223,7 +1223,7 @@ static int probe_hfsplus(struct blkid_probe *probe,
off = (alloc_first_block * 512) +
(embed_first_block * alloc_block_size);
buf = get_buffer(probe, off + (id->bim_kboff * 1024),
- sizeof(sbd));
+ sizeof(*sbd));
if (!buf)
return 1;
--
1.8.5.rc3.362.gdf10213
It's highly unlikely after five seconds that zero blocks would have
been written, but let's silence the Coverity warning.
Addresses-Coverity-ID: 1147780
Signed-off-by: "Theodore Ts'o" <[email protected]>
---
misc/e2image.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/misc/e2image.c b/misc/e2image.c
index ac62ffe..6c51137 100644
--- a/misc/e2image.c
+++ b/misc/e2image.c
@@ -634,7 +634,7 @@ more_blocks:
bscount = print_progress(total_written,
meta_blocks_count);
duration = time(NULL) - start_time;
- if (duration > 5) {
+ if (duration > 5 && total_written) {
time_t est = (duration * meta_blocks_count /
total_written) - duration;
char buff[30];
--
1.8.5.rc3.362.gdf10213
On 1/2/14, 11:29 PM, Theodore Ts'o wrote:
> Addresses-Coverity-ID: #1147783
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
Thanks for fixing these -
Reviewed-by: Eric Sandeen <[email protected]>
(Aside: Seems odd that -n emits "Writing block XXX" when it's
not actually writing anything, but *shrug*)
-Eric
> ---
> misc/e2image.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/misc/e2image.c b/misc/e2image.c
> index ab6a4fa..253fad1 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -174,7 +174,7 @@ static void generic_write(int fd, void *buf, int blocksize, blk64_t block)
> printf(_("Writing block %llu\n"), (unsigned long long) block);
> if (fd != 1)
> seek_relative(fd, blocksize);
> - return;
> + goto free_and_return;
> }
> count = write(fd, buf, blocksize);
> if (count != blocksize) {
> @@ -191,6 +191,7 @@ static void generic_write(int fd, void *buf, int blocksize, blk64_t block)
>
> exit(1);
> }
> +free_and_return:
> if (free_buf)
> ext2fs_free_mem(&buf);
> }
>
On 1/2/14, 11:29 PM, Theodore Ts'o wrote:
> Dividing a floating point number by zero is undefined in C. It
> happens to work with gcc/glibc, but it's not something that's
> guaranteed.
>
> Addresses-Coverity-ID: #1147781
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
Reviewed-by: Eric Sandeen <[email protected]>
> ---
> misc/e2image.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/misc/e2image.c b/misc/e2image.c
> index 253fad1..ac62ffe 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -704,14 +704,15 @@ more_blocks:
> if (show_progress) {
> time_t duration = time(NULL) - start_time;
> char buff[30];
> - while (bscount--)
> - fputc('\b', stderr);
> + fputc('\r', stderr);
> strftime(buff, 30, "%T", gmtime(&duration));
> - fprintf(stderr, _("\b\b\b\b\b\b\b\bCopied %llu / %llu "
> - "blocks (%d%%) in %s at %.2f MB/s \n"),
> - total_written, meta_blocks_count,
> - calc_percent(total_written, meta_blocks_count), buff,
> - calc_rate(total_written, fs->blocksize, duration));
> + fprintf(stderr, _("Copied %llu / %llu blocks (%d%%) in %s "),
> + total_written, meta_blocks_count,
> + calc_percent(total_written, meta_blocks_count), buff);
> + if (duration)
> + fprintf(stderr, _("at %.2f MB/s"),
> + calc_rate(total_written, fs->blocksize, duration));
> + fputs(" \n", stderr);
> }
> #ifdef HAVE_FTRUNCATE64
> if (sparse) {
>
On 1/2/14, 11:29 PM, Theodore Ts'o wrote:
> It's highly unlikely after five seconds that zero blocks would have
> been written, but let's silence the Coverity warning.
>
> Addresses-Coverity-ID: 1147780
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
Reviewed-by: Eric Sandeen <[email protected]>
> ---
> misc/e2image.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/misc/e2image.c b/misc/e2image.c
> index ac62ffe..6c51137 100644
> --- a/misc/e2image.c
> +++ b/misc/e2image.c
> @@ -634,7 +634,7 @@ more_blocks:
> bscount = print_progress(total_written,
> meta_blocks_count);
> duration = time(NULL) - start_time;
> - if (duration > 5) {
> + if (duration > 5 && total_written) {
> time_t est = (duration * meta_blocks_count /
> total_written) - duration;
> char buff[30];
>
On 1/2/14, 11:29 PM, Theodore Ts'o wrote:
> Addresses-Coverity-Bug: #709510
>
> Signed-off-by: "Theodore Ts'o" <[email protected]>
Heh, that's been around a while!
Reviewed-by: Eric Sandeen <[email protected]>
> ---
> lib/blkid/probe.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/lib/blkid/probe.c b/lib/blkid/probe.c
> index bd31fe0..6f74bd4 100644
> --- a/lib/blkid/probe.c
> +++ b/lib/blkid/probe.c
> @@ -1003,7 +1003,7 @@ static int probe_udf(struct blkid_probe *probe,
> (block sizes larger than 2K will be null padded) */
> for (bs = 1; bs < 16; bs++) {
> isosb = (struct iso_volume_descriptor *)
> - get_buffer(probe, bs*2048+32768, sizeof(isosb));
> + get_buffer(probe, bs*2048+32768, sizeof(*isosb));
> if (!isosb)
> return 1;
> if (isosb->vd_id[0])
> @@ -1015,7 +1015,7 @@ static int probe_udf(struct blkid_probe *probe,
> if (j > 1) {
> isosb = (struct iso_volume_descriptor *)
> get_buffer(probe, j*bs*2048+32768,
> - sizeof(isosb));
> + sizeof(*isosb));
> if (!isosb)
> return 1;
> }
> @@ -1223,7 +1223,7 @@ static int probe_hfsplus(struct blkid_probe *probe,
> off = (alloc_first_block * 512) +
> (embed_first_block * alloc_block_size);
> buf = get_buffer(probe, off + (id->bim_kboff * 1024),
> - sizeof(sbd));
> + sizeof(*sbd));
> if (!buf)
> return 1;
>
>