From e1fb96f07a06f3133f54702ed8706493989789fe Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 9 Oct 2007 03:50:20 +0000 Subject: [PATCH] Fix bug #21280. Thanks to John Darrington for review. * automake.mk: Add new file. * bugs/overwrite-input-file.sh: New test. * file-name.c (create_stream): New function. * por-file-writer.c (pfm_open_writer): Use fh_open to open the file handle before creating the file, to ensure that we don't truncate a file that we're reading. Make code easier to read by using create_stream. --- src/data/ChangeLog | 13 +++ src/data/file-name.c | 26 ++++++ src/data/file-name.h | 3 + src/data/por-file-writer.c | 47 +++++------ src/data/sys-file-writer.c | 49 +++++------ tests/ChangeLog | 8 ++ tests/automake.mk | 1 + tests/bugs/overwrite-input-file.sh | 125 +++++++++++++++++++++++++++++ 8 files changed, 216 insertions(+), 56 deletions(-) create mode 100755 tests/bugs/overwrite-input-file.sh diff --git a/src/data/ChangeLog b/src/data/ChangeLog index 819cb127..a8f897ef 100644 --- a/src/data/ChangeLog +++ b/src/data/ChangeLog @@ -1,3 +1,16 @@ +2007-10-08 Ben Pfaff + + Fix bug #21280. Thanks to John Darrington for review. + + * file-name.c (create_stream): New function. + + * por-file-writer.c (pfm_open_writer): Use fh_open to open the + file handle before creating the file, to ensure that we don't + truncate a file that we're reading. Make code easier to read by + using create_stream. + + * sys-file-write.c (sfm_open_writer): Ditto. + 2007-10-01 Ben Pfaff Fix bug #21192. Thanks to John Darrington for review. diff --git a/src/data/file-name.c b/src/data/file-name.c index f3b16817..f8ad11d0 100644 --- a/src/data/file-name.c +++ b/src/data/file-name.c @@ -20,8 +20,10 @@ #include #include +#include #include #include +#include #include "intprops.h" #include "minmax.h" @@ -320,6 +322,30 @@ fn_close (const char *fn, FILE *f) return fclose (f); } +/* Creates a new file named FN with the given PERMISSIONS bits, + and returns a stream for it or a null pointer on failure. + MODE should be "w" or "wb". */ +FILE * +create_stream (const char *fn, const char *mode, mode_t permissions) +{ + int fd; + FILE *stream; + + fd = open (fn, O_WRONLY | O_CREAT | O_TRUNC, permissions); + if (fd < 0) + return NULL; + + stream = fdopen (fd, mode); + if (stream == NULL) + { + int save_errno = errno; + close (fd); + errno = save_errno; + } + + return stream; +} + #if !(defined _WIN32 || defined __WIN32__) /* A file's identity. */ struct file_identity diff --git a/src/data/file-name.h b/src/data/file-name.h index d18ce22a..5564280b 100644 --- a/src/data/file-name.h +++ b/src/data/file-name.h @@ -20,6 +20,7 @@ #include #include #include +#include /* Search path for configuration files. */ extern const char *config_path; @@ -43,6 +44,8 @@ const char *fn_getenv_default (const char *variable, const char *def); FILE *fn_open (const char *fn, const char *mode); int fn_close (const char *fn, FILE *file); +FILE *create_stream (const char *fn, const char *mode, mode_t permissions); + struct file_identity *fn_get_identity (const char *file_name); void fn_free_identity (struct file_identity *); int fn_compare_file_identities (const struct file_identity *, diff --git a/src/data/por-file-writer.c b/src/data/por-file-writer.c index 27f7c5c4..f3a1101c 100644 --- a/src/data/por-file-writer.c +++ b/src/data/por-file-writer.c @@ -19,20 +19,19 @@ #include #include -#include #include #include #include #include #include #include -#include #include #include #include #include #include +#include #include #include #include @@ -109,30 +108,31 @@ pfm_open_writer (struct file_handle *fh, struct dictionary *dict, { struct pfm_writer *w = NULL; mode_t mode; - int fd; + FILE *file; size_t i; + /* Open file handle. */ + if (!fh_open (fh, FH_REF_FILE, "portable file", "we")) + return NULL; + /* Create file. */ mode = S_IRUSR | S_IRGRP | S_IROTH; if (opts.create_writeable) mode |= S_IWUSR | S_IWGRP | S_IWOTH; - fd = open (fh_get_file_name (fh), O_WRONLY | O_CREAT | O_TRUNC, mode); - if (fd < 0) - goto open_error; - - /* Open file handle. */ - if (!fh_open (fh, FH_REF_FILE, "portable file", "we")) - goto error; + file = create_stream (fh_get_file_name (fh), "w", mode); + if (file == NULL) + { + fh_close (fh, "portable file", "we"); + msg (ME, _("An error occurred while opening \"%s\" for writing " + "as a portable file: %s."), + fh_get_file_name (fh), strerror (errno)); + return NULL; + } /* Initialize data structures. */ w = xmalloc (sizeof *w); w->fh = fh; - w->file = fdopen (fd, "w"); - if (w->file == NULL) - { - close (fd); - goto open_error; - } + w->file = file; w->lc = 0; w->var_cnt = 0; @@ -165,19 +165,12 @@ pfm_open_writer (struct file_handle *fh, struct dictionary *dict, write_documents (w, dict); buf_write (w, "F", 1); if (ferror (w->file)) - goto error; + { + close_writer (w); + return NULL; + } return casewriter_create (dict_get_next_value_idx (dict), &por_file_casewriter_class, w); - - error: - close_writer (w); - return NULL; - - open_error: - msg (ME, _("An error occurred while opening \"%s\" for writing " - "as a portable file: %s."), - fh_get_file_name (fh), strerror (errno)); - goto error; } /* Write NBYTES starting at BUF to the portable file represented by diff --git a/src/data/sys-file-writer.c b/src/data/sys-file-writer.c index d58fd835..21d89a0d 100644 --- a/src/data/sys-file-writer.c +++ b/src/data/sys-file-writer.c @@ -21,12 +21,10 @@ #include #include -#include #include #include #include #include -#include #include #include @@ -42,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -152,7 +151,7 @@ sfm_open_writer (struct file_handle *fh, struct dictionary *d, { struct sfm_writer *w = NULL; mode_t mode; - int fd; + FILE *file; int idx; int i; @@ -164,22 +163,27 @@ sfm_open_writer (struct file_handle *fh, struct dictionary *d, opts.version = 3; } - /* Create file. */ + /* Open file handle as an exclusive writer. */ + if (!fh_open (fh, FH_REF_FILE, "system file", "we")) + return NULL; + + /* Create the file on disk. */ mode = S_IRUSR | S_IRGRP | S_IROTH; if (opts.create_writeable) mode |= S_IWUSR | S_IWGRP | S_IWOTH; - fd = open (fh_get_file_name (fh), O_WRONLY | O_CREAT | O_TRUNC, mode); - if (fd < 0) - goto open_error; - - /* Open file handle. */ - if (!fh_open (fh, FH_REF_FILE, "system file", "we")) - goto error; + file = create_stream (fh_get_file_name (fh), "w", mode); + if (file == NULL) + { + msg (ME, _("Error opening \"%s\" for writing as a system file: %s."), + fh_get_file_name (fh), strerror (errno)); + fh_close (fh, "system file", "we"); + return NULL; + } /* Create and initialize writer. */ w = xmalloc (sizeof *w); w->fh = fh; - w->file = fdopen (fd, "w"); + w->file = file; w->compress = opts.compress; w->case_cnt = 0; @@ -193,13 +197,6 @@ sfm_open_writer (struct file_handle *fh, struct dictionary *d, w->segment_cnt = sfm_dictionary_to_sfm_vars (d, &w->sfm_vars, &w->sfm_var_cnt); - /* Check that file create succeeded. */ - if (w->file == NULL) - { - close (fd); - goto open_error; - } - /* Write the file header. */ write_header (w, d); @@ -236,19 +233,13 @@ sfm_open_writer (struct file_handle *fh, struct dictionary *d, write_int (w, 0); if (write_error (w)) - goto error; + { + close_writer (w); + return NULL; + } return casewriter_create (dict_get_next_value_idx (d), &sys_file_casewriter_class, w); - - error: - close_writer (w); - return NULL; - - open_error: - msg (ME, _("Error opening \"%s\" for writing as a system file: %s."), - fh_get_file_name (fh), strerror (errno)); - goto error; } /* Returns value of X truncated to two least-significant digits. */ diff --git a/tests/ChangeLog b/tests/ChangeLog index 3a912d56..c98f53e2 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,11 @@ +2007-10-08 Ben Pfaff + + Bug #21280. Thanks to John Darrington for review. + + * automake.mk: Add new file. + + * bugs/overwrite-input-file.sh: New test. + 2007-09-23 Ben Pfaff Bug #21111. Reviewed by John Darrington. diff --git a/tests/automake.mk b/tests/automake.mk index 25eb1ab9..5e6e2369 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -106,6 +106,7 @@ dist_TESTS = \ tests/bugs/list-overflow.sh \ tests/bugs/match-files-scratch.sh \ tests/bugs/multipass.sh \ + tests/bugs/overwrite-input-file.sh \ tests/bugs/random.sh \ tests/bugs/signals.sh \ tests/bugs/t-test-with-temp.sh \ diff --git a/tests/bugs/overwrite-input-file.sh b/tests/bugs/overwrite-input-file.sh new file mode 100755 index 00000000..56d569cb --- /dev/null +++ b/tests/bugs/overwrite-input-file.sh @@ -0,0 +1,125 @@ +#!/bin/sh + +# This program tests for a bug that caused SAVE to the file currently +# being read with GET to truncate the save file to zero length, and +# similarly for IMPORT/EXPORT. + + +TEMPDIR=/tmp/pspp-tst-$$ +TESTFILE=$TEMPDIR/`basename $0`.sps + +# ensure that top_builddir are absolute +if [ -z "$top_builddir" ] ; then top_builddir=. ; fi +if [ -z "$top_srcdir" ] ; then top_srcdir=. ; fi +top_builddir=`cd $top_builddir; pwd` +PSPP=$top_builddir/src/ui/terminal/pspp + +# ensure that top_srcdir is absolute +top_srcdir=`cd $top_srcdir; pwd` + +STAT_CONFIG_PATH=$top_srcdir/config +export STAT_CONFIG_PATH + + +cleanup() +{ + cd / + rm -rf $TEMPDIR +} + + +fail() +{ + echo $activity + echo FAILED + cleanup; + exit 1; +} + + +no_result() +{ + echo $activity + echo NO RESULT; + cleanup; + exit 2; +} + +pass() +{ + cleanup; + exit 0; +} + +mkdir -p $TEMPDIR + +cd $TEMPDIR + +activity="create program 1" +cat > $TESTFILE < $TESTFILE < $TESTFILE <