From: Ben Pfaff Date: Sat, 3 Nov 2007 22:00:03 +0000 (+0000) Subject: Allow output files to overwrite input files (bug #21280). Thanks to X-Git-Tag: v0.6.0~207 X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?p=pspp-builds.git;a=commitdiff_plain;h=707848060e414fe93458834446dd7cdbf800667f Allow output files to overwrite input files (bug #21280). Thanks to John Darrington for review. --- diff --git a/ChangeLog b/ChangeLog index 5768f68b..696c526d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * Smake (GNULIB_MODULES): Add fatal-signal, tempname modules. + 2007-11-02 Ben Pfaff * Smake (GNULIB_MODULES): Add isfinite, round modules. diff --git a/Smake b/Smake index aaf1ff90..43536f9d 100644 --- a/Smake +++ b/Smake @@ -12,6 +12,7 @@ GNULIB_MODULES = \ crypto/md4 \ dirname \ exit \ + fatal-signal \ fpieee \ fprintf-posix \ full-read \ @@ -61,6 +62,7 @@ GNULIB_MODULES = \ strtol \ strtoul \ sys_stat \ + tempname \ tmpfile \ trunc \ unistd \ diff --git a/src/data/ChangeLog b/src/data/ChangeLog index 6f116701..0353c638 100644 --- a/src/data/ChangeLog +++ b/src/data/ChangeLog @@ -1,3 +1,103 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * file-handle-def.c: Separate locking of files for input and for + output, to allow both to take place at once. Also, distinguish a + file handle from the identity of the underlying file, because the + identity of a file changes over time and the file handle can't + represent two different identities. + (struct file_handle): Remove `next', `open_cnt', `deleted', + `type', `open_mode', `aux', `identity' members. Change `id' from + char array to char *. Add `ref_cnt' member. + (file_handle_from_ll) New function. + (file_handles) Removed. + (named_handles) New variable. + (fh_init) Initialize named_handles. + (fh_done) Remove name from all named_handles. + (free_handle) Updated for modified struct file_handle. + (unname_handle) New function. + (fh_ref) New function. + (fh_from_file_name) Removed. + (fh_unref) New function. + (fh_unname) New function. + (fh_from_id) Rewritten. + (create_handle) Updated for modified struct file_handle. + (fh_inline_file) Increment the handle's ref count. + (fh_create_file) Updated for modified struct file_handle. + (fh_create_scratch) Ditto. + (fh_free) Removed. + (mode_name) Removed. + (fh_open) Removed. + (fh_close) Removed. + (fh_is_open) Removed. + (fh_get_id) Updated for modified struct file_handle. + (fh_get_default_handle) Increment the handle's ref count. + (fh_set_default_handle) Handle ref counts. + (struct fh_lock) New structure. + (locks) New static var. + (fh_lock) New function. + (fh_unlock) New function. + (fh_lock_get_aux) New function. + (fh_lock_set_aux) New function. + (fh_is_locked) New function. + (make_key) New function. + (free_key) New function. + (compare_fh_locks) New function. + (hash_fh_lock) New function. + + * file-handle-def.h (enum fh_access) New enum. + + * file-name.c: Made file_identity the same in all supported + environments. + (struct file_identity): New `name' member. + (fn_get_identity): For a file that doesn't exist, get the + dev/inode of its directory plus its name. If even the directory + doesn't exist, just use its name. Merge the Windows + implementation into the Unix one. + (fn_compare_file_identities): Rewritten. Merge the Windows + implementation into the Unix one. + (fn_hash_identity): New function. + + * make-file.c (struct replace_file): New structure. + (all_files): New static var. + (replace_file_start): New function. + (replace_file_commit): New function. + (replace_file_abort): New function. + (free_replace_file): New function. + (unlink_replace_files): New function. + + * por-file-reader.c (struct pfm_reader): Add `lock' member. + (close_reader): Unlock file. + (pfm_open_reader): Lock file. + + * por-file-writer.c (struct pfm_writer): Add fh_lock, replace_file + members. + (pfm_open_writer): Lock file and prepare for its replacement. + (close_writer): Unlock file. + + * scratch-handle.h (struct scratch_handle): Add unique_id so that + different generations of a scratch file can be distinguished. + + * scratch-reader.c (scratch_reader_open): Verify that the file is + a scratch file. + + * scratch-writer.c (struct scratch_writer): Add `lock' and `dict', + remove scratch_handle member. + (scratch_writer_open): Lock handle. Prepare to replace handle + data, instead of doing it immediately. + (scratch_writer_casewriter_destroy): Replace handle data and + unlock handle. + + * sys-file-reader.c (struct sfm_reader): Add `lock' member. + (sfm_open_reader): Lock file. + (close_reader): Unlock file. + + * sys-file-writer.c (struct sfm_writer): Add fh_lock, replace_file + members. + (sfm_open_writer): Lock file and prepare for its replacement. + (close_writer): Unlock file. + 2007-11-02 Ben Pfaff * data-out.c (output_number): Use isfinite (from C99) instead of diff --git a/src/data/file-handle-def.c b/src/data/file-handle-def.c index 1dea25a5..87255915 100644 --- a/src/data/file-handle-def.c +++ b/src/data/file-handle-def.c @@ -24,6 +24,8 @@ #include #include +#include +#include #include #include #include @@ -35,25 +37,17 @@ #include "gettext.h" #define _(msgid) gettext (msgid) -/* (headers) */ - /* File handle. */ struct file_handle { - struct file_handle *next; /* Next in global list. */ - int open_cnt; /* 0=not open, otherwise # of openers. */ - bool deleted; /* Destroy handle when open_cnt goes to 0? */ - - char id[LONG_NAME_LEN + 1]; /* Identifier token; empty string if none. */ + struct ll ll; /* Element in global list. */ + size_t ref_cnt; /* Number of references. */ + char *id; /* Identifier token, NULL if none. */ char *name; /* User-friendly identifying name. */ - const char *type; /* If open, type of file. */ - char open_mode[3]; /* "[rw][se]". */ - void *aux; /* Aux data pointer for owner if any. */ enum fh_referent referent; /* What the file handle refers to. */ /* FH_REF_FILE only. */ char *file_name; /* File name as provided by user. */ - struct file_identity *identity; /* For checking file identity. */ enum fh_mode mode; /* File mode. */ /* FH_REF_FILE and FH_REF_INLINE only. */ @@ -64,8 +58,14 @@ struct file_handle struct scratch_handle *sh; /* Scratch file data. */ }; -/* List of all handles. */ -static struct file_handle *file_handles; +static struct file_handle * +file_handle_from_ll (struct ll *ll) +{ + return ll_data (ll, struct file_handle, ll); +} + +/* List of all named handles. */ +static struct ll_list named_handles; /* Default file handle for DATA LIST, REREAD, REPEATING DATA commands. */ @@ -75,92 +75,108 @@ static struct file_handle *default_handle; static struct file_handle *inline_file; static struct file_handle *create_handle (const char *id, - const char *name, enum fh_referent); + char *name, enum fh_referent); +static void free_handle (struct file_handle *); +static void unname_handle (struct file_handle *); /* File handle initialization routine. */ void fh_init (void) { - inline_file = create_handle ("INLINE", "INLINE", FH_REF_INLINE); + ll_init (&named_handles); + inline_file = create_handle ("INLINE", xstrdup ("INLINE"), FH_REF_INLINE); inline_file->record_width = 80; inline_file->tab_width = 8; } +/* Removes all named file handles from the global list. */ +void +fh_done (void) +{ + while (!ll_is_empty (&named_handles)) + unname_handle (file_handle_from_ll (ll_head (&named_handles))); +} + /* Free HANDLE and remove it from the global list. */ static void free_handle (struct file_handle *handle) { /* Remove handle from global list. */ - if (file_handles == handle) - file_handles = handle->next; - else - { - struct file_handle *iter = file_handles; - while (iter->next != handle) - iter = iter->next; - iter->next = handle->next; - } + if (handle->id != NULL) + ll_remove (&handle->ll); /* Free data. */ + free (handle->id); free (handle->name); free (handle->file_name); - fn_free_identity (handle->identity); scratch_handle_destroy (handle->sh); free (handle); } -/* Frees all the file handles. */ -void -fh_done (void) +/* Make HANDLE unnamed, so that it can no longer be referenced by + name. The caller must hold a reference to HANDLE, which is + not affected by this function. */ +static void +unname_handle (struct file_handle *handle) { - while (file_handles != NULL) - free_handle (file_handles); + assert (handle->id != NULL); + free (handle->id); + handle->id = NULL; + ll_remove (&handle->ll); + + /* Drop the reference held by the named_handles table. */ + fh_unref (handle); } -/* Returns the handle with the given ID, or a null pointer if - there is none. */ +/* Increments HANDLE's reference count and returns HANDLE. */ struct file_handle * -fh_from_id (const char *id) +fh_ref (struct file_handle *handle) { - struct file_handle *iter; - - for (iter = file_handles; iter != NULL; iter = iter->next) - if (!iter->deleted && !strcasecmp (id, iter->id)) - return iter; - return NULL; + assert (handle->ref_cnt > 0); + handle->ref_cnt++; + return handle; } -/* Returns the handle for the file named FILE_NAME, - or a null pointer if none exists. - Different names for the same file (e.g. "x" and "./x") are - considered equivalent. */ -struct file_handle * -fh_from_file_name (const char *file_name) +/* Decrements HANDLE's reference count. + If the reference count drops to 0, HANDLE is destroyed. */ +void +fh_unref (struct file_handle *handle) { - struct file_identity *identity; - struct file_handle *iter; - - /* First check for a file with the same identity. */ - identity = fn_get_identity (file_name); - if (identity != NULL) + if (handle != NULL) { - for (iter = file_handles; iter != NULL; iter = iter->next) - if (!iter->deleted - && iter->referent == FH_REF_FILE - && iter->identity != NULL - && !fn_compare_file_identities (identity, iter->identity)) - { - fn_free_identity (identity); - return iter; - } - fn_free_identity (identity); + assert (handle->ref_cnt > 0); + if (--handle->ref_cnt == 0) + free_handle (handle); } +} + +/* Make HANDLE unnamed, so that it can no longer be referenced by + name. The caller must hold a reference to HANDLE, which is + not affected by this function. + + This function ignores a null pointer as input. It has no + effect on the inline handle, which is always named INLINE.*/ +void +fh_unname (struct file_handle *handle) +{ + assert (handle->ref_cnt > 1); + if (handle != fh_inline_file () && handle->id != NULL) + unname_handle (handle); +} + +/* Returns the handle with the given ID, or a null pointer if + there is none. */ +struct file_handle * +fh_from_id (const char *id) +{ + struct file_handle *handle; - /* Then check for a file with the same name. */ - for (iter = file_handles; iter != NULL; iter = iter->next) - if (!iter->deleted - && iter->referent == FH_REF_FILE && !strcmp (file_name, iter->file_name)) - return iter; + ll_for_each (handle, struct file_handle, ll, &named_handles) + if (!strcasecmp (id, handle->id)) + { + handle->ref_cnt++; + return handle; + } return NULL; } @@ -172,20 +188,22 @@ fh_from_file_name (const char *file_name) The new handle is not fully initialized. The caller is responsible for completing its initialization. */ static struct file_handle * -create_handle (const char *id, const char *handle_name, - enum fh_referent referent) +create_handle (const char *id, char *handle_name, enum fh_referent referent) { struct file_handle *handle = xzalloc (sizeof *handle); - assert (id == NULL || fh_from_id (id) == NULL); - handle->next = file_handles; - handle->open_cnt = 0; - handle->deleted = false; - str_copy_trunc (handle->id, sizeof handle->id, id != NULL ? id : ""); - handle->name = xstrdup (handle_name); - handle->type = NULL; - handle->aux = NULL; + + handle->ref_cnt = 1; + handle->id = id != NULL ? xstrdup (id) : NULL; + handle->name = handle_name; handle->referent = referent; - file_handles = handle; + + if (id != NULL) + { + assert (fh_from_id (id) == NULL); + ll_push_tail (&named_handles, &handle->ll); + handle->ref_cnt++; + } + return handle; } @@ -195,13 +213,14 @@ create_handle (const char *id, const char *handle_name, struct file_handle * fh_inline_file (void) { + fh_ref (inline_file); return inline_file; } -/* Creates a new file handle with the given ID, which may be - null. If it is non-null, it must be unique among existing - file identifiers. The new handle is associated with file - FILE_NAME and the given PROPERTIES. */ +/* Creates and returns a new file handle with the given ID, which + may be null. If it is non-null, it must be unique among + existing file identifiers. The new handle is associated with + file FILE_NAME and the given PROPERTIES. */ struct file_handle * fh_create_file (const char *id, const char *file_name, const struct fh_properties *properties) @@ -209,12 +228,9 @@ fh_create_file (const char *id, const char *file_name, char *handle_name; struct file_handle *handle; - handle_name = id != NULL ? (char *) id : xasprintf ("\"%s\"", file_name); + handle_name = id != NULL ? xstrdup (id) : xasprintf ("\"%s\"", file_name); handle = create_handle (id, handle_name, FH_REF_FILE); - if (id == NULL) - free (handle_name); handle->file_name = xstrdup (file_name); - handle->identity = fn_get_identity (file_name); handle->mode = properties->mode; handle->record_width = properties->record_width; handle->tab_width = properties->tab_width; @@ -228,8 +244,7 @@ struct file_handle * fh_create_scratch (const char *id) { struct file_handle *handle; - assert (id != NULL); - handle = create_handle (id, id, FH_REF_SCRATCH); + handle = create_handle (id, xstrdup (id), FH_REF_SCRATCH); handle->sh = NULL; return handle; } @@ -243,142 +258,6 @@ fh_default_properties (void) return &default_properties; } -/* Deletes FH from the global list of file handles. Afterward, - attempts to search for it will fail. Unless the file handle - is currently open, it will be destroyed; otherwise, it will be - destroyed later when it is closed. - Normally needed only if a file_handle needs to be re-assigned. - Otherwise, just let fh_done() destroy the handle. */ -void -fh_free (struct file_handle *handle) -{ - if (handle == fh_inline_file () || handle == NULL || handle->deleted) - return; - handle->deleted = true; - - if (handle == default_handle) - default_handle = fh_inline_file (); - - if (handle->open_cnt == 0) - free_handle (handle); -} - -/* Returns an English description of MODE, - which is in the format of the MODE argument to fh_open(). */ -static const char * -mode_name (const char *mode) -{ - assert (mode != NULL); - assert (mode[0] == 'r' || mode[0] == 'w'); - - return mode[0] == 'r' ? "reading" : "writing"; -} - -/* Tries to open handle H with the given TYPE and MODE. - - H's referent type must be one of the bits in MASK. The caller - must verify this ahead of time; we simply assert it here. - - TYPE is the sort of file, e.g. "system file". Only one given - type of access is allowed on a given file handle at once. - If successful, a reference to TYPE is retained, so it should - probably be a string literal. - - MODE combines the read or write mode with the sharing mode. - The first character is 'r' for read, 'w' for write. The - second character is 's' to permit sharing, 'e' to require - exclusive access. - - Returns the address of a void * that the caller can use for - data specific to the file handle if successful, or a null - pointer on failure. For exclusive access modes the void * - will always be a null pointer at return. In shared access - modes the void * will necessarily be null only if no other - sharers are active. */ -void ** -fh_open (struct file_handle *h, enum fh_referent mask UNUSED, - const char *type, const char *mode) -{ - assert (h != NULL); - assert ((fh_get_referent (h) & mask) != 0); - assert (type != NULL); - assert (mode != NULL); - assert (mode[0] == 'r' || mode[0] == 'w'); - assert (mode[1] == 's' || mode[1] == 'e'); - assert (mode[2] == '\0'); - - if (h->open_cnt != 0) - { - if (strcmp (h->type, type)) - { - msg (SE, _("Can't open %s as a %s because it is " - "already open as a %s."), - fh_get_name (h), type, h->type); - return NULL; - } - else if (strcmp (h->open_mode, mode)) - { - msg (SE, _("Can't open %s as a %s for %s because it is " - "already open for %s."), - fh_get_name (h), type, mode_name (mode), - mode_name (h->open_mode)); - return NULL; - } - else if (h->open_mode[1] == 'e') - { - msg (SE, _("Can't re-open %s as a %s for %s."), - fh_get_name (h), type, mode_name (mode)); - return NULL; - } - } - else - { - h->type = type; - strcpy (h->open_mode, mode); - assert (h->aux == NULL); - } - h->open_cnt++; - - return &h->aux; -} - -/* Closes file handle H, which must have been open for the - specified TYPE and MODE of access provided to fh_open(). - Returns zero if the file is now closed, nonzero if it is still - open due to another reference. - - After fh_close() returns zero for a handle, it is unsafe to - reference that file handle again in any way, because its - storage may have been freed. */ -int -fh_close (struct file_handle *h, const char *type, const char *mode) -{ - assert (h != NULL); - assert (h->open_cnt > 0); - assert (type != NULL); - assert (!strcmp (type, h->type)); - assert (mode != NULL); - assert (!strcmp (mode, h->open_mode)); - - if (--h->open_cnt == 0) - { - h->type = NULL; - h->aux = NULL; - if (h->deleted) - free_handle (h); - return 0; - } - return 1; -} - -/* Is the file open? BEGIN DATA...END DATA uses this to detect - whether the inline file is actually in use. */ -bool -fh_is_open (const struct file_handle *handle) -{ - return handle->open_cnt > 0; -} - /* Returns the identifier that may be used in syntax to name the given HANDLE, which takes the form of a PSPP identifier. If HANDLE has no identifier, returns a null pointer. @@ -387,7 +266,7 @@ fh_is_open (const struct file_handle *handle) const char * fh_get_id (const struct file_handle *handle) { - return handle->id[0] != '\0' ? handle->id : NULL; + return handle->id; } /* Returns a user-friendly string to identify the given HANDLE. @@ -446,7 +325,7 @@ fh_get_tab_width (const struct file_handle *handle) /* Returns the scratch file handle associated with HANDLE. Applicable to only FH_REF_SCRATCH files. */ struct scratch_handle * -fh_get_scratch_handle (struct file_handle *handle) +fh_get_scratch_handle (const struct file_handle *handle) { assert (handle->referent == FH_REF_SCRATCH); return handle->sh; @@ -465,7 +344,7 @@ fh_set_scratch_handle (struct file_handle *handle, struct scratch_handle *sh) struct file_handle * fh_get_default_handle (void) { - return default_handle ? default_handle : fh_inline_file (); + return default_handle ? fh_ref (default_handle) : fh_inline_file (); } /* Sets NEW_DEFAULT_HANDLE as the default handle. */ @@ -474,5 +353,235 @@ fh_set_default_handle (struct file_handle *new_default_handle) { assert (new_default_handle == NULL || (new_default_handle->referent & (FH_REF_INLINE | FH_REF_FILE))); + if (default_handle != NULL) + fh_unref (default_handle); default_handle = new_default_handle; + if (default_handle != NULL) + fh_ref (default_handle); +} + +/* Information about a file handle's readers or writers. */ +struct fh_lock + { + /* Hash key. */ + enum fh_referent referent; /* Type of underlying file. */ + union + { + struct file_identity *file; /* FH_REF_FILE only. */ + unsigned int unique_id; /* FH_REF_SCRATCH only. */ + } + u; + enum fh_access access; /* Type of file access. */ + + /* Number of openers. */ + size_t open_cnt; + + /* Applicable only when open_cnt > 0. */ + bool exclusive; /* No other openers allowed? */ + const char *type; /* Human-readable type of file. */ + void *aux; /* Owner's auxiliary data. */ + }; + +/* Hash table of all active locks. */ +static struct hsh_table *locks; + +static void make_key (struct fh_lock *, const struct file_handle *, + enum fh_access); +static void free_key (struct fh_lock *); +static int compare_fh_locks (const void *, const void *, const void *); +static unsigned int hash_fh_lock (const void *, const void *); + +/* Tries to lock handle H for the given kind of ACCESS and TYPE + of file. Returns a pointer to a struct fh_lock if successful, + otherwise a null pointer. + + H's referent type must be one of the bits in MASK. The caller + must verify this ahead of time; we simply assert it here. + + TYPE is the sort of file, e.g. "system file". Only one type + of access is allowed on a given file at a time for reading, + and similarly for writing. If successful, a reference to TYPE + is retained, so it should probably be a string literal. + + ACCESS specifies whether the lock is for reading or writing. + EXCLUSIVE is true to require exclusive access, false to allow + sharing with other accessors. Exclusive read access precludes + other readers, but not writers; exclusive write access + precludes other writers, but not readers. A sharable read or + write lock precludes reader or writers, respectively, of a + different TYPE. + + A lock may be associated with auxiliary data. See + fh_lock_get_aux and fh_lock_set_aux for more details. */ +struct fh_lock * +fh_lock (struct file_handle *h, enum fh_referent mask UNUSED, + const char *type, enum fh_access access, bool exclusive) +{ + struct fh_lock key, *lock; + void **lockp; + + assert ((fh_get_referent (h) & mask) != 0); + assert (access == FH_ACC_READ || access == FH_ACC_WRITE); + + if (locks == NULL) + locks = hsh_create (0, compare_fh_locks, hash_fh_lock, NULL, NULL); + + make_key (&key, h, access); + lockp = hsh_probe (locks, &key); + if (*lockp == NULL) + { + lock = *lockp = xmalloc (sizeof *lock); + *lock = key; + lock->open_cnt = 1; + lock->exclusive = exclusive; + lock->type = type; + lock->aux = NULL; + } + else + { + free_key (&key); + + lock = *lockp; + if (strcmp (lock->type, type)) + { + if (access == FH_ACC_READ) + msg (SE, _("Can't read from %s as a %s because it is " + "already being read as a %s."), + fh_get_name (h), gettext (type), gettext (lock->type)); + else + msg (SE, _("Can't write to %s as a %s because it is " + "already being written as a %s."), + fh_get_name (h), gettext (type), gettext (lock->type)); + return NULL; + } + else if (exclusive || lock->exclusive) + { + msg (SE, _("Can't re-open %s as a %s."), + fh_get_name (h), gettext (type)); + return NULL; + } + lock->open_cnt++; + } + + return lock; +} + +/* Releases LOCK that was acquired with fh_lock. + Returns true if LOCK is still locked, because other clients + also had it locked. + + Returns false if LOCK has now been destroyed. In this case + the caller must ensure that any auxiliary data associated with + LOCK is destroyed, to avoid a memory leak. The caller must + obtain a pointer to the auxiliary data, e.g. via + fh_lock_get_aux *before* calling fh_unlock (because it yields + undefined behavior to call fh_lock_get_aux on a destroyed + lock). */ +bool +fh_unlock (struct fh_lock *lock) +{ + if (lock != NULL) + { + assert (lock->open_cnt > 0); + if (--lock->open_cnt == 0) + { + hsh_delete (locks, lock); + free_key (lock); + free (lock); + return false; + } + } + return true; +} + +/* Returns auxiliary data for LOCK. + + Auxiliary data is shared by every client that holds LOCK (for + an exclusive lock, this is a single client). To avoid leaks, + auxiliary data must be released before LOCK is destroyed. */ +void * +fh_lock_get_aux (const struct fh_lock *lock) +{ + return lock->aux; +} + +/* Sets the auxiliary data for LOCK to AUX. */ +void +fh_lock_set_aux (struct fh_lock *lock, void *aux) +{ + lock->aux = aux; +} + +/* Returns true if HANDLE is locked for the given type of ACCESS, + false otherwise. */ +bool +fh_is_locked (const struct file_handle *handle, enum fh_access access) +{ + struct fh_lock key; + bool is_locked; + + make_key (&key, handle, access); + is_locked = hsh_find (locks, &key) != NULL; + free_key (&key); + + return is_locked; +} + +/* Initializes the key fields in LOCK for looking up or inserting + handle H for the given kind of ACCESS. */ +static void +make_key (struct fh_lock *lock, const struct file_handle *h, + enum fh_access access) +{ + lock->referent = fh_get_referent (h); + lock->access = access; + if (lock->referent == FH_REF_FILE) + lock->u.file = fn_get_identity (fh_get_file_name (h)); + else if (lock->referent == FH_REF_SCRATCH) + { + struct scratch_handle *sh = fh_get_scratch_handle (h); + lock->u.unique_id = sh != NULL ? sh->unique_id : 0; + } +} + +/* Frees the key fields in LOCK. */ +static void +free_key (struct fh_lock *lock) +{ + if (lock->referent == FH_REF_FILE) + fn_free_identity (lock->u.file); +} + +/* Compares the key fields in struct fh_lock objects A and B and + returns a strcmp()-type result. */ +static int +compare_fh_locks (const void *a_, const void *b_, const void *aux UNUSED) +{ + const struct fh_lock *a = a_; + const struct fh_lock *b = b_; + + if (a->referent != b->referent) + return a->referent < b->referent ? -1 : 1; + else if (a->access != b->access) + return a->access < b->access ? -1 : 1; + else if (a->referent == FH_REF_FILE) + return fn_compare_file_identities (a->u.file, b->u.file); + else if (a->referent == FH_REF_SCRATCH) + return (a->u.unique_id < b->u.unique_id ? -1 + : a->u.unique_id > b->u.unique_id); + else + return 0; +} + +/* Returns a hash value for LOCK. */ +static unsigned int +hash_fh_lock (const void *lock_, const void *aux UNUSED) +{ + const struct fh_lock *lock = lock_; + unsigned int hash = hsh_hash_int ((lock->referent << 3) | lock->access); + if (lock->referent == FH_REF_FILE) + hash ^= fn_hash_identity (lock->u.file); + else if (lock->referent == FH_REF_SCRATCH) + hash ^= hsh_hash_int (lock->u.unique_id); + return hash; } diff --git a/src/data/file-handle-def.h b/src/data/file-handle-def.h index 0a793095..79f0c52a 100644 --- a/src/data/file-handle-def.h +++ b/src/data/file-handle-def.h @@ -37,6 +37,13 @@ enum fh_mode FH_MODE_BINARY /* Fixed-length records. */ }; +/* Ways to access a file. */ +enum fh_access + { + FH_ACC_READ, /* Read from it. */ + FH_ACC_WRITE /* Write to it. */ + }; + /* Properties of a file handle. */ struct fh_properties { @@ -55,8 +62,10 @@ struct file_handle *fh_create_file (const char *handle_name, struct file_handle *fh_create_scratch (const char *handle_name); const struct fh_properties *fh_default_properties (void); -/* Delete file handle from global list. */ -void fh_free (struct file_handle *); +/* Reference management. */ +struct file_handle *fh_ref (struct file_handle *); +void fh_unref (struct file_handle *); +void fh_unname (struct file_handle *); /* Finding file handles. */ struct file_handle *fh_from_id (const char *handle_name); @@ -77,14 +86,16 @@ size_t fh_get_record_width (const struct file_handle *); size_t fh_get_tab_width (const struct file_handle *); /* Properties of FH_REF_SCRATCH file handles. */ -struct scratch_handle *fh_get_scratch_handle (struct file_handle *); +struct scratch_handle *fh_get_scratch_handle (const struct file_handle *); void fh_set_scratch_handle (struct file_handle *, struct scratch_handle *); -/* Opening and closing file handles. */ -void **fh_open (struct file_handle *, enum fh_referent mask, - const char *type, const char *mode); -int fh_close (struct file_handle *, const char *type, const char *mode); -bool fh_is_open (const struct file_handle *); +/* Mutual exclusion for access . */ +struct fh_lock *fh_lock (struct file_handle *, enum fh_referent mask, + const char *type, enum fh_access, bool exclusive); +bool fh_unlock (struct fh_lock *); +void *fh_lock_get_aux (const struct fh_lock *); +void fh_lock_set_aux (struct fh_lock *, void *aux); +bool fh_is_locked (const struct file_handle *, enum fh_access); /* Default file handle for DATA LIST, REREAD, REPEATING DATA commands. */ diff --git a/src/data/file-name.c b/src/data/file-name.c index 82279f77..2fd79428 100644 --- a/src/data/file-name.c +++ b/src/data/file-name.c @@ -30,8 +30,9 @@ #include "dirname.h" #include "xmalloca.h" -#include #include +#include +#include #include #include #include @@ -348,12 +349,24 @@ create_stream (const char *fn, const char *mode, mode_t permissions) return stream; } -#if !(defined _WIN32 || defined __WIN32__) -/* A file's identity. */ +/* A file's identity: + + - For a file that exists, this is its device and inode. + + - For a file that does not exist, but which has a directory + name that exists, this is the device and inode of the + directory, plus the file's base name. + + - For a file that does not exist and has a nonexistent + directory, this is the file name. + + Windows doesn't have inode numbers, so we just use the name + there. */ struct file_identity { dev_t device; /* Device number. */ ino_t inode; /* Inode number. */ + char *name; /* File name, where needed, otherwise NULL. */ }; /* Returns a pointer to a dynamically allocated structure whose @@ -365,61 +378,41 @@ struct file_identity struct file_identity * fn_get_identity (const char *file_name) { - struct stat s; + struct file_identity *identity = xmalloc (sizeof *identity); - if (stat (file_name, &s) == 0) +#if !(defined _WIN32 || defined __WIN32__) + struct stat s; + if (lstat (file_name, &s) == 0) { - struct file_identity *identity = xmalloc (sizeof *identity); identity->device = s.st_dev; identity->inode = s.st_ino; - return identity; + identity->name = NULL; } else - return NULL; -} - -/* Frees IDENTITY obtained from fn_get_identity(). */ -void -fn_free_identity (struct file_identity *identity) -{ - free (identity); -} - -/* Compares A and B, returning a strcmp()-type result. */ -int -fn_compare_file_identities (const struct file_identity *a, - const struct file_identity *b) -{ - assert (a != NULL); - assert (b != NULL); - if (a->device != b->device) - return a->device < b->device ? -1 : 1; - else - return a->inode < b->inode ? -1 : a->inode > b->inode; -} + { + char *dir = dir_name (file_name); + if (last_component (file_name) != NULL && stat (dir, &s) == 0) + { + identity->device = s.st_dev; + identity->inode = s.st_ino; + identity->name = base_name (file_name); + } + else + { + identity->device = 0; + identity->inode = 0; + identity->name = xstrdup (file_name); + } + free (dir); + } #else /* Windows */ -/* A file's identity. */ -struct file_identity -{ - char *normalized_file_name; /* File's normalized name. */ -}; - -/* Returns a pointer to a dynamically allocated structure whose - value can be used to tell whether two files are actually the - same file. Returns a null pointer if no information about the - file is available, perhaps because it does not exist. The - caller is responsible for freeing the structure with - fn_free_identity() when finished. */ -struct file_identity * -fn_get_identity (const char *file_name) -{ - struct file_identity *identity = xmalloc (sizeof *identity); char cname[PATH_MAX]; - - if (GetFullPathName (file_name, sizeof cname, cname, NULL)) - identity->normalized_file_name = xstrdup (cname); - else - identity->normalized_file_name = xstrdup (file_name); + int ok = GetFullPathName (file_name, sizeof cname, cname, NULL); + identity->device = 0; + identity->inode = 0; + identity->name = xstrdup (ok ? cname : file_name); + str_lowercase (identity->file_name); +#endif /* Windows */ return identity; } @@ -430,7 +423,7 @@ fn_free_identity (struct file_identity *identity) { if (identity != NULL) { - free (identity->normalized_file_name); + free (identity->name); free (identity); } } @@ -440,6 +433,22 @@ int fn_compare_file_identities (const struct file_identity *a, const struct file_identity *b) { - return strcasecmp (a->normalized_file_name, b->normalized_file_name); + if (a->device != b->device) + return a->device < b->device ? -1 : 1; + else if (a->inode != b->inode) + return a->inode < b->inode ? -1 : 1; + else if (a->name != NULL) + return b->name != NULL ? strcmp (a->name, b->name) : 1; + else + return b->name != NULL ? -1 : 0; +} + +/* Returns a hash value for IDENTITY. */ +unsigned int +fn_hash_identity (const struct file_identity *identity) +{ + unsigned int hash = identity->device ^ identity->inode; + if (identity->name != NULL) + hash ^= hsh_hash_string (identity->name); + return hash; } -#endif /* Windows */ diff --git a/src/data/file-name.h b/src/data/file-name.h index 5564280b..b4231d2f 100644 --- a/src/data/file-name.h +++ b/src/data/file-name.h @@ -50,5 +50,6 @@ 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 *, const struct file_identity *); +unsigned int fn_hash_identity (const struct file_identity *); #endif /* file-name.h */ diff --git a/src/data/make-file.c b/src/data/make-file.c index 051905b8..f8719dc6 100644 --- a/src/data/make-file.c +++ b/src/data/make-file.c @@ -16,14 +16,19 @@ #include #include +#include #include #include #include #include -#include "file-name.h" -#include "make-file.h" + +#include +#include +#include #include +#include "fatal-signal.h" +#include "tempname.h" #include "xalloc.h" #include "gettext.h" @@ -105,7 +110,197 @@ make_unique_file_stream (FILE **fp, char **file_name) return 1; } + +struct replace_file + { + struct ll ll; + char *file_name; + char *tmp_name; + }; + +static struct ll_list all_files = LL_INITIALIZER (all_files); + +static void free_replace_file (struct replace_file *); +static void unlink_replace_files (void); + +struct replace_file * +replace_file_start (const char *file_name, const char *mode, + mode_t permissions, FILE **fp, char **tmp_name) +{ + static bool registered; + struct stat s; + struct replace_file *rf; + int fd; + + /* If FILE_NAME represents a special file, write to it directly + instead of trying to replace it. */ + if (stat (file_name, &s) == 0 && !S_ISREG (s.st_mode)) + { + /* Open file descriptor. */ + fd = open (file_name, O_WRONLY); + if (fd < 0) + { + msg (ME, _("Opening %s for writing: %s."), + file_name, strerror (errno)); + return NULL; + } + + /* Open file as stream. */ + *fp = fdopen (fd, mode); + if (*fp == NULL) + { + msg (ME, _("Opening stream for %s: %s."), + file_name, strerror (errno)); + close (fd); + return NULL; + } + + rf = xmalloc (sizeof *rf); + rf->file_name = NULL; + rf->tmp_name = xstrdup (file_name); + if (tmp_name != NULL) + *tmp_name = rf->tmp_name; + return rf; + } + + if (!registered) + { + at_fatal_signal (unlink_replace_files); + registered = true; + } + block_fatal_signals (); + + rf = xmalloc (sizeof *rf); + rf->file_name = xstrdup (file_name); + for (;;) + { + /* Generate unique temporary file name. */ + rf->tmp_name = xasprintf ("%s.tmpXXXXXX", file_name); + if (gen_tempname (rf->tmp_name, GT_NOCREATE) < 0) + { + msg (ME, _("Creating temporary file to replace %s: %s."), + rf->file_name, strerror (errno)); + goto error; + } + + /* Create file by that name. */ + fd = open (rf->tmp_name, O_WRONLY | O_CREAT | O_EXCL, permissions); + if (fd >= 0) + break; + if (errno != EEXIST) + { + msg (ME, _("Creating temporary file %s: %s."), + rf->tmp_name, strerror (errno)); + goto error; + } + free (rf->tmp_name); + } + + + /* Open file as stream. */ + *fp = fdopen (fd, mode); + if (*fp == NULL) + { + msg (ME, _("Opening stream for temporary file %s: %s."), + rf->tmp_name, strerror (errno)); + close (fd); + unlink (rf->tmp_name); + goto error; + } + + /* Register file for deletion. */ + ll_push_head (&all_files, &rf->ll); + unblock_fatal_signals (); + + if (tmp_name != NULL) + *tmp_name = rf->tmp_name; + + return rf; + +error: + unblock_fatal_signals (); + free_replace_file (rf); + *fp = NULL; + if (tmp_name != NULL) + *tmp_name = NULL; + return NULL; +} +bool +replace_file_commit (struct replace_file *rf) +{ + bool ok = true; + if (rf->file_name != NULL) + { + int save_errno; + block_fatal_signals (); + ok = rename (rf->tmp_name, rf->file_name) == 0; + save_errno = errno; + ll_remove (&rf->ll); + unblock_fatal_signals (); + if (!ok) + msg (ME, _("Replacing %s by %s: %s."), + rf->tmp_name, rf->file_name, strerror (save_errno)); + } + else + { + /* Special file: no temporary file to rename. */ + } + free_replace_file (rf); + + return ok; +} + +bool +replace_file_abort (struct replace_file *rf) +{ + bool ok = true; + + if (rf->file_name != NULL) + { + int save_errno; + + block_fatal_signals (); + ok = unlink (rf->tmp_name) == 0; + save_errno = errno; + ll_remove (&rf->ll); + unblock_fatal_signals (); + + if (!ok) + msg (ME, _("Removing %s: %s."), rf->tmp_name, strerror (save_errno)); + } + else + { + /* Special file: no temporary file to unlink. */ + } + free_replace_file (rf); + + return ok; +} + +static void +free_replace_file (struct replace_file *rf) +{ + free (rf->file_name); + free (rf->tmp_name); + free (rf); +} + +static void +unlink_replace_files (void) +{ + struct replace_file *rf; + + block_fatal_signals (); + ll_for_each (rf, struct replace_file, ll, &all_files) + { + /* We don't free_replace_file(RF) because calling free is unsafe + from an asynchronous signal handler. */ + unlink (rf->tmp_name); + fflush (stdout); + } + unblock_fatal_signals (); +} diff --git a/src/data/make-file.h b/src/data/make-file.h index ce56758e..19d33ebe 100644 --- a/src/data/make-file.h +++ b/src/data/make-file.h @@ -29,4 +29,31 @@ int make_temp_file (int *fd, char **file_name); responsible for freeing *FILE_NAME. */ int make_unique_file_stream (FILE **fp, char **file_name) ; + +/* Prepares to atomically replace a (potentially) existing file + by a new file, by creating a temporary file with the given + PERMISSIONS bits in the same directory as *FILE_NAME. + + Special files are an exception: they are not atomically + replaced but simply opened for writing. + + If successful, stores the temporary file's name in *TMP_NAME + and a stream for it opened according to MODE (which should be + "w" or "wb") in *FP. Returns a ticket that can be used to + commit or abort the file replacement. If neither action has + yet been taken, program termination via signal will cause + *TMP_FILE to be unlinked. + + The caller is responsible for closing *FP, but *TMP_NAME is + owned by the callee. */ +struct replace_file *replace_file_start (const char *file_name, + const char *mode, mode_t permissions, + FILE **fp, char **tmp_name); + +/* Commits or aborts the replacement of a (potentially) existing + file by a new file, using the ticket returned by + replace_file_start. Returns success. */ +bool replace_file_commit (struct replace_file *); +bool replace_file_abort (struct replace_file *); + #endif /* make-file.h */ diff --git a/src/data/por-file-reader.c b/src/data/por-file-reader.c index 37ae2335..668a8429 100644 --- a/src/data/por-file-reader.c +++ b/src/data/por-file-reader.c @@ -65,6 +65,7 @@ struct pfm_reader jmp_buf bail_out; /* longjmp() target for error handling. */ struct file_handle *fh; /* File handle. */ + struct fh_lock *lock; /* Read lock for file. */ FILE *file; /* File stream. */ int line_length; /* Number of characters so far on this line. */ char cc; /* Current character. */ @@ -157,8 +158,8 @@ close_reader (struct pfm_reader *r) r->file = NULL; } - if (r->fh != NULL) - fh_close (r->fh, "portable file", "rs"); + fh_unlock (r->lock); + fh_unref (r->fh); ok = r->ok; pool_destroy (r->pool); @@ -241,15 +242,14 @@ pfm_open_reader (struct file_handle *fh, struct dictionary **dict, struct pfm_reader *volatile r = NULL; *dict = dict_create (); - if (!fh_open (fh, FH_REF_FILE, "portable file", "rs")) - goto error; /* Create and initialize reader. */ pool = pool_create (); r = pool_alloc (pool, sizeof *r); r->pool = pool; - r->fh = fh; - r->file = fn_open (fh_get_file_name (r->fh), "rb"); + r->fh = fh_ref (fh); + r->lock = NULL; + r->file = NULL; r->line_length = 0; r->weight_index = -1; r->trans = NULL; @@ -257,11 +257,16 @@ pfm_open_reader (struct file_handle *fh, struct dictionary **dict, r->widths = NULL; r->value_cnt = 0; r->ok = true; - if (setjmp (r->bail_out)) goto error; - /* Check that file open succeeded. */ + /* Lock file. */ + r->lock = fh_lock (fh, FH_REF_FILE, "portable file", FH_ACC_READ, false); + if (r->lock == NULL) + goto error; + + /* Open file. */ + r->file = fn_open (fh_get_file_name (r->fh), "rb"); if (r->file == NULL) { msg (ME, _("An error occurred while opening \"%s\" for reading " diff --git a/src/data/por-file-writer.c b/src/data/por-file-writer.c index 111043a8..3d62078e 100644 --- a/src/data/por-file-writer.c +++ b/src/data/por-file-writer.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -56,7 +57,9 @@ struct pfm_writer { struct file_handle *fh; /* File handle. */ + struct fh_lock *lock; /* Lock on file handle. */ FILE *file; /* File stream. */ + struct replace_file *rf; /* Ticket for replacing output file. */ int lc; /* Number of characters on this line so far. */ @@ -108,32 +111,14 @@ pfm_open_writer (struct file_handle *fh, struct dictionary *dict, { struct pfm_writer *w = NULL; mode_t mode; - 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; - 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 = file; - + w->fh = fh_ref (fh); + w->lock = NULL; + w->file = NULL; + w->rf = NULL; w->lc = 0; w->var_cnt = 0; w->vars = NULL; @@ -156,6 +141,24 @@ pfm_open_writer (struct file_handle *fh, struct dictionary *dict, w->digits = DBL_DIG; } + /* Lock file. */ + w->lock = fh_lock (fh, FH_REF_FILE, "portable file", FH_ACC_WRITE, true); + if (w->lock == NULL) + goto error; + + /* Create file. */ + mode = S_IRUSR | S_IRGRP | S_IROTH; + if (opts.create_writeable) + mode |= S_IWUSR | S_IWGRP | S_IWOTH; + w->rf = replace_file_start (fh_get_file_name (fh), "w", mode, + &w->file, NULL); + if (w->rf == NULL) + { + msg (ME, _("Error opening \"%s\" for writing as a portable file: %s."), + fh_get_file_name (fh), strerror (errno)); + goto error; + } + /* Write file header. */ write_header (w); write_version_data (w); @@ -165,12 +168,13 @@ pfm_open_writer (struct file_handle *fh, struct dictionary *dict, write_documents (w, dict); buf_write (w, "F", 1); if (ferror (w->file)) - { - close_writer (w); - return NULL; - } + goto error; return casewriter_create (dict_get_next_value_idx (dict), &por_file_casewriter_class, w); + +error: + close_writer (w); + return NULL; } /* Write NBYTES starting at BUF to the portable file represented by @@ -491,9 +495,13 @@ close_writer (struct pfm_writer *w) if (!ok) msg (ME, _("An I/O error occurred writing portable file \"%s\"."), fh_get_file_name (w->fh)); + + if (ok ? !replace_file_commit (w->rf) : !replace_file_abort (w->rf)) + ok = false; } - fh_close (w->fh, "portable file", "we"); + fh_unlock (w->lock); + fh_unref (w->fh); free (w->vars); free (w); diff --git a/src/data/scratch-handle.h b/src/data/scratch-handle.h index 1d638ef5..c7775f4d 100644 --- a/src/data/scratch-handle.h +++ b/src/data/scratch-handle.h @@ -22,6 +22,7 @@ /* A scratch file. */ struct scratch_handle { + unsigned int unique_id; /* Identifies this scratch file. */ struct dictionary *dictionary; /* Dictionary. */ struct casereader *casereader; /* Cases. */ }; diff --git a/src/data/scratch-reader.c b/src/data/scratch-reader.c index d82486fe..3aa1768d 100644 --- a/src/data/scratch-reader.c +++ b/src/data/scratch-reader.c @@ -16,15 +16,16 @@ #include -#include "scratch-reader.h" +#include #include -#include "dictionary.h" -#include "file-handle-def.h" -#include "scratch-handle.h" #include #include +#include +#include +#include +#include #include #include "xalloc.h" @@ -41,8 +42,11 @@ scratch_reader_open (struct file_handle *fh, struct dictionary **dict) { struct scratch_handle *sh; - if (!fh_open (fh, FH_REF_SCRATCH, "scratch file", "rs")) - return NULL; + /* We don't bother doing fh_lock or fh_ref on the file handle, + as there's no advantage in this case, and doing these would + require us to keep track of the "struct file_handle" and + "struct fh_lock" and undo our work later. */ + assert (fh_get_referent (fh) == FH_REF_SCRATCH); sh = fh_get_scratch_handle (fh); if (sh == NULL || sh->casereader == NULL) diff --git a/src/data/scratch-writer.c b/src/data/scratch-writer.c index 4e2929a2..70a65ce9 100644 --- a/src/data/scratch-writer.c +++ b/src/data/scratch-writer.c @@ -37,9 +37,10 @@ /* A scratch file writer. */ struct scratch_writer { - struct scratch_handle *handle; /* Underlying scratch handle. */ struct file_handle *fh; /* Underlying file handle. */ - struct case_map *compactor; /* Compacts into handle->dictionary. */ + struct fh_lock *lock; /* Exclusive access to file handle. */ + struct dictionary *dict; /* Dictionary for subwriter. */ + struct case_map *compactor; /* Compacts into dictionary. */ struct casewriter *subwriter; /* Data output. */ }; @@ -53,47 +54,34 @@ struct casewriter * scratch_writer_open (struct file_handle *fh, const struct dictionary *dictionary) { - struct scratch_handle *sh; struct scratch_writer *writer; - struct dictionary *scratch_dict; - struct case_map *compactor; struct casewriter *casewriter; + struct fh_lock *lock; size_t dict_value_cnt; - if (!fh_open (fh, FH_REF_SCRATCH, "scratch file", "we")) + /* Get exclusive write access to handle. */ + lock = fh_lock (fh, FH_REF_SCRATCH, "scratch file", FH_ACC_WRITE, true); + if (lock == NULL) return NULL; - /* Destroy previous contents of handle. */ - sh = fh_get_scratch_handle (fh); - if (sh != NULL) - scratch_handle_destroy (sh); + /* Create writer. */ + writer = xmalloc (sizeof *writer); + writer->lock = lock; + writer->fh = fh_ref (fh); - /* Copy the dictionary and compact if needed. */ - scratch_dict = dict_clone (dictionary); - dict_delete_scratch_vars (scratch_dict); - if (dict_count_values (scratch_dict, 0) - < dict_get_next_value_idx (scratch_dict)) + writer->dict = dict_clone (dictionary); + dict_delete_scratch_vars (writer->dict); + if (dict_count_values (writer->dict, 0) + < dict_get_next_value_idx (writer->dict)) { - compactor = case_map_to_compact_dict (scratch_dict, 0); - dict_compact_values (scratch_dict); + writer->compactor = case_map_to_compact_dict (writer->dict, 0); + dict_compact_values (writer->dict); } else - compactor = NULL; - dict_value_cnt = dict_get_next_value_idx (scratch_dict); - - /* Create new contents. */ - sh = xmalloc (sizeof *sh); - sh->dictionary = scratch_dict; - sh->casereader = NULL; - - /* Create writer. */ - writer = xmalloc (sizeof *writer); - writer->handle = sh; - writer->fh = fh; - writer->compactor = compactor; + writer->compactor = NULL; + dict_value_cnt = dict_get_next_value_idx (writer->dict); writer->subwriter = autopaging_writer_create (dict_value_cnt); - fh_set_scratch_handle (fh, sh); casewriter = casewriter_create (dict_value_cnt, &scratch_writer_casewriter_class, writer); taint_propagate (casewriter_get_taint (writer->subwriter), @@ -122,11 +110,32 @@ scratch_writer_casewriter_write (struct casewriter *w UNUSED, void *writer_, static void scratch_writer_casewriter_destroy (struct casewriter *w UNUSED, void *writer_) { + static unsigned int next_unique_id = 0x12345678; + struct scratch_writer *writer = writer_; struct casereader *reader = casewriter_make_reader (writer->subwriter); if (!casereader_error (reader)) - writer->handle->casereader = reader; - fh_close (writer->fh, "scratch file", "we"); + { + /* Destroy previous contents of handle. */ + struct scratch_handle *sh = fh_get_scratch_handle (writer->fh); + if (sh != NULL) + scratch_handle_destroy (sh); + + /* Create new contents. */ + sh = xmalloc (sizeof *sh); + sh->unique_id = ++next_unique_id; + sh->dictionary = writer->dict; + sh->casereader = reader; + fh_set_scratch_handle (writer->fh, sh); + } + else + { + casereader_destroy (reader); + dict_destroy (writer->dict); + } + + fh_unlock (writer->lock); + fh_unref (writer->fh); free (writer); } diff --git a/src/data/sys-file-reader.c b/src/data/sys-file-reader.c index de9ae0d6..7c0312d2 100644 --- a/src/data/sys-file-reader.c +++ b/src/data/sys-file-reader.c @@ -66,6 +66,7 @@ struct sfm_reader /* File state. */ struct file_handle *fh; /* File handle. */ + struct fh_lock *lock; /* Mutual exclusion for file handle. */ FILE *file; /* File stream. */ bool error; /* I/O or corruption error? */ size_t value_cnt; /* Number of "union value"s in struct case. */ @@ -179,39 +180,38 @@ sfm_open_reader (struct file_handle *fh, struct dictionary **dict, int claimed_oct_cnt; int rec_type; - if (!fh_open (fh, FH_REF_FILE, "system file", "rs")) - return NULL; - *dict = dict_create (); /* Create and initialize reader. */ r = pool_create_container (struct sfm_reader, pool); - r->fh = fh; - r->file = fn_open (fh_get_file_name (fh), "rb"); + r->fh = fh_ref (fh); + r->lock = NULL; + r->file = NULL; r->error = false; r->oct_cnt = 0; r->has_long_var_names = false; r->opcode_idx = sizeof r->opcodes; + r->lock = fh_lock (fh, FH_REF_FILE, "system file", FH_ACC_READ, false); + if (r->lock == NULL) + goto error; + + r->file = fn_open (fh_get_file_name (fh), "rb"); + if (r->file == NULL) + { + msg (ME, _("Error opening \"%s\" for reading as a system file: %s."), + fh_get_file_name (r->fh), strerror (errno)); + goto error; + } + /* Initialize info. */ if (info == NULL) info = &local_info; memset (info, 0, sizeof *info); if (setjmp (r->bail_out)) - { - close_reader (r); - dict_destroy (*dict); - *dict = NULL; - return NULL; - } + goto error; - if (r->file == NULL) - { - msg (ME, _("Error opening \"%s\" for reading as a system file: %s."), - fh_get_file_name (r->fh), strerror (errno)); - longjmp (r->bail_out, 1); - } /* Read header. */ read_header (r, *dict, &weight_idx, &claimed_oct_cnt, info); @@ -305,6 +305,12 @@ sfm_open_reader (struct file_handle *fh, struct dictionary **dict, (NULL, r->value_cnt, r->case_cnt == -1 ? CASENUMBER_MAX: r->case_cnt, &sys_file_casereader_class, r); + +error: + close_reader (r); + dict_destroy (*dict); + *dict = NULL; + return NULL; } /* Closes a system file after we're done with it. @@ -329,8 +335,8 @@ close_reader (struct sfm_reader *r) r->file = NULL; } - if (r->fh != NULL) - fh_close (r->fh, "system file", "rs"); + fh_unlock (r->lock); + fh_unref (r->fh); error = r->error; pool_destroy (r->pool); diff --git a/src/data/sys-file-writer.c b/src/data/sys-file-writer.c index a20b9fc1..d25d5e8d 100644 --- a/src/data/sys-file-writer.c +++ b/src/data/sys-file-writer.c @@ -40,6 +40,7 @@ #include #include #include +#include #include #include #include @@ -62,7 +63,9 @@ struct sfm_writer { struct file_handle *fh; /* File handle. */ + struct fh_lock *lock; /* Mutual exclusion for file. */ FILE *file; /* File stream. */ + struct replace_file *rf; /* Ticket for replacing output file. */ bool compress; /* 1=compressed, 0=not compressed. */ casenumber case_cnt; /* Number of cases written so far. */ @@ -148,9 +151,8 @@ struct casewriter * sfm_open_writer (struct file_handle *fh, struct dictionary *d, struct sfm_write_options opts) { - struct sfm_writer *w = NULL; + struct sfm_writer *w; mode_t mode; - FILE *file; int idx; int i; @@ -162,27 +164,12 @@ sfm_open_writer (struct file_handle *fh, struct dictionary *d, opts.version = 3; } - /* 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; - 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 = file; + w->fh = fh_ref (fh); + w->lock = NULL; + w->file = NULL; + w->rf = NULL; w->compress = opts.compress; w->case_cnt = 0; @@ -196,6 +183,24 @@ 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); + /* Open file handle as an exclusive writer. */ + w->lock = fh_lock (fh, FH_REF_FILE, "system file", FH_ACC_WRITE, true); + if (w->lock == NULL) + goto error; + + /* Create the file on disk. */ + mode = S_IRUSR | S_IRGRP | S_IROTH; + if (opts.create_writeable) + mode |= S_IWUSR | S_IWGRP | S_IWOTH; + w->rf = replace_file_start (fh_get_file_name (fh), "wb", mode, + &w->file, NULL); + if (w->rf == NULL) + { + msg (ME, _("Error opening \"%s\" for writing as a system file: %s."), + fh_get_file_name (fh), strerror (errno)); + goto error; + } + /* Write the file header. */ write_header (w, d); @@ -239,6 +244,10 @@ sfm_open_writer (struct file_handle *fh, struct dictionary *d, return casewriter_create (dict_get_next_value_idx (d), &sys_file_casewriter_class, w); + +error: + close_writer (w); + return NULL; } /* Returns value of X truncated to two least-significant digits. */ @@ -724,9 +733,13 @@ close_writer (struct sfm_writer *w) if (!ok) msg (ME, _("An I/O error occurred writing system file \"%s\"."), fh_get_file_name (w->fh)); + + if (ok ? !replace_file_commit (w->rf) : !replace_file_abort (w->rf)) + ok = false; } - fh_close (w->fh, "system file", "we"); + fh_unlock (w->lock); + fh_unref (w->fh); free (w->sfm_vars); free (w); diff --git a/src/language/data-io/ChangeLog b/src/language/data-io/ChangeLog index 7982916a..b58f9f7c 100644 --- a/src/language/data-io/ChangeLog +++ b/src/language/data-io/ChangeLog @@ -1,3 +1,36 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * data-list.c (cmd_data_list): Manage file handle reference + counts. + + * data-reader.c (struct dfm_reader): Add `lock' member. + (dfm_close_reader): Simplify, as reference counting is now + separate from locking. + (dfm_open_reader): Lock file. + + * data-writer.c (struct dfm_writer): Add fh_lock, replace_file + members. + (dfm_open_writer): Lock file and prepare for replacement. + (dfm_close_writer): Unlock file and replace it. + + * file-handle.q (cmd_close_file_handle): Use fh_unname. + (fh_parse): Don't distinguish existing handles for a given file + name from new ones. Manage file handle reference counts. + + * get.c (parse_read_command): Manage file handle reference counts. + (parse_write_command): Ditto. + (mtf_close_all_files): Ditto. + + * inpt-pgm.c (cmd_reread): Manage file handle reference counts. + + * print-space.c (cmd_print_space): Manage file handle reference + counts. + + * print.c (internal_cmd_print): Manage file handle reference + counts. + 2007-11-03 John Darrington * get.c: Add GET DATA command variant. diff --git a/src/language/data-io/data-list.c b/src/language/data-io/data-list.c index 6ca829fc..3ee524d5 100644 --- a/src/language/data-io/data-list.c +++ b/src/language/data-io/data-list.c @@ -120,7 +120,7 @@ cmd_data_list (struct lexer *lexer, struct dataset *ds) struct dictionary *dict; struct data_list_pgm *dls; int table = -1; /* Print table if nonzero, -1=undecided. */ - struct file_handle *fh = fh_inline_file (); + struct file_handle *fh = NULL; struct pool *tmp_pool; bool ok; @@ -143,6 +143,7 @@ cmd_data_list (struct lexer *lexer, struct dataset *ds) if (lex_match_id (lexer, "FILE")) { lex_match (lexer, '='); + fh_unref (fh); fh = fh_parse (lexer, FH_REF_FILE | FH_REF_INLINE); if (fh == NULL) goto error; @@ -244,6 +245,8 @@ cmd_data_list (struct lexer *lexer, struct dataset *ds) } } + if (fh == NULL) + fh = fh_inline_file (); fh_set_default_handle (fh); if (dls->type == -1) @@ -292,10 +295,12 @@ cmd_data_list (struct lexer *lexer, struct dataset *ds) } pool_destroy (tmp_pool); + fh_unref (fh); return CMD_SUCCESS; error: + fh_unref (fh); data_list_trns_free (dls); return CMD_CASCADING_FAILURE; } diff --git a/src/language/data-io/data-reader.c b/src/language/data-io/data-reader.c index e2b76e7e..6113bf74 100644 --- a/src/language/data-io/data-reader.c +++ b/src/language/data-io/data-reader.c @@ -55,6 +55,7 @@ enum dfm_reader_flags struct dfm_reader { struct file_handle *fh; /* File handle. */ + struct fh_lock *lock; /* Mutual exclusion lock for file. */ struct msg_locator where; /* Current location in data file. */ struct string line; /* Current line. */ struct string scratch; /* Extra line buffer. */ @@ -69,24 +70,18 @@ struct dfm_reader void dfm_close_reader (struct dfm_reader *r) { - int still_open; - bool is_inline; - char *file_name; - if (r == NULL) return; - is_inline = r->fh == fh_inline_file (); - file_name = is_inline ? NULL : xstrdup (fh_get_file_name (r->fh)); - still_open = fh_close (r->fh, "data file", "rs"); - if (still_open) + if (fh_unlock (r->lock)) { - free (file_name); + /* File is still locked by another client. */ return; } - if (!is_inline) - fn_close (file_name, r->file); + /* This was the last client, so close the underlying file. */ + if (fh_get_referent (r->fh) != FH_REF_INLINE) + fn_close (fh_get_file_name (r->fh), r->file); else { /* Skip any remaining data on the inline file. */ @@ -98,10 +93,10 @@ dfm_close_reader (struct dfm_reader *r) } } + fh_unref (r->fh); ds_destroy (&r->line); ds_destroy (&r->scratch); free (r); - free (file_name); } /* Opens the file designated by file handle FH for reading as a @@ -113,22 +108,26 @@ struct dfm_reader * dfm_open_reader (struct file_handle *fh, struct lexer *lexer) { struct dfm_reader *r; - void **rp; + struct fh_lock *lock; - rp = fh_open (fh, FH_REF_FILE | FH_REF_INLINE, "data file", "rs"); - if (rp == NULL) + lock = fh_lock (fh, FH_REF_FILE | FH_REF_INLINE, "data file", + FH_ACC_READ, false); + if (lock == NULL) return NULL; - if (*rp != NULL) - return *rp; + + r = fh_lock_get_aux (lock); + if (r != NULL) + return r; r = xmalloc (sizeof *r); - r->fh = fh; - r->lexer = lexer ; + r->fh = fh_ref (fh); + r->lock = lock; + r->lexer = lexer; ds_init_empty (&r->line); ds_init_empty (&r->scratch); r->flags = DFM_ADVANCE; r->eof_cnt = 0; - if (fh != fh_inline_file ()) + if (fh_get_referent (fh) != FH_REF_INLINE) { r->where.file_name = fh_get_file_name (fh); r->where.line_number = 0; @@ -137,12 +136,13 @@ dfm_open_reader (struct file_handle *fh, struct lexer *lexer) { msg (ME, _("Could not open \"%s\" for reading as a data file: %s."), fh_get_file_name (r->fh), strerror (errno)); - fh_close (fh,"data file", "rs"); + fh_unlock (r->lock); + fh_unref (fh); free (r); return NULL; } } - *rp = r; + fh_lock_set_aux (lock, r); return r; } @@ -430,7 +430,7 @@ cmd_begin_data (struct lexer *lexer, struct dataset *ds) struct dfm_reader *r; bool ok; - if (!fh_is_open (fh_inline_file ())) + if (!fh_is_locked (fh_inline_file (), FH_ACC_READ)) { msg (SE, _("This command is not valid here since the current " "input program does not access the inline file.")); diff --git a/src/language/data-io/data-writer.c b/src/language/data-io/data-writer.c index 55b6737b..b1b955c8 100644 --- a/src/language/data-io/data-writer.c +++ b/src/language/data-io/data-writer.c @@ -21,8 +21,10 @@ #include #include #include +#include #include +#include #include #include #include @@ -38,7 +40,9 @@ struct dfm_writer { struct file_handle *fh; /* File handle. */ + struct fh_lock *lock; /* Exclusive access to file. */ FILE *file; /* Associated file. */ + struct replace_file *rf; /* Atomic file replacement support. */ }; /* Opens a file handle for writing as a data file. */ @@ -46,31 +50,33 @@ struct dfm_writer * dfm_open_writer (struct file_handle *fh) { struct dfm_writer *w; - void **aux; + struct fh_lock *lock; - aux = fh_open (fh, FH_REF_FILE, "data file", "ws"); - if (aux == NULL) + lock = fh_lock (fh, FH_REF_FILE, "data file", FH_ACC_WRITE, false); + if (lock == NULL) return NULL; - if (*aux != NULL) - return *aux; - w = *aux = xmalloc (sizeof *w); - w->fh = fh; - w->file = fn_open (fh_get_file_name (w->fh), "wb"); - - if (w->file == NULL) + w = fh_lock_get_aux (lock); + if (w != NULL) + return w; + + w = xmalloc (sizeof *w); + w->fh = fh_ref (fh); + w->lock = lock; + w->rf = replace_file_start (fh_get_file_name (w->fh), "wb", + (S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP + | S_IROTH | S_IWOTH), &w->file, NULL); + if (w->rf == NULL) { msg (ME, _("An error occurred while opening \"%s\" for writing " "as a data file: %s."), fh_get_file_name (w->fh), strerror (errno)); - goto error; + dfm_close_writer (w); + return NULL; } + fh_lock_set_aux (lock, w); return w; - - error: - dfm_close_writer (w); - return NULL; } /* Returns false if an I/O error occurred on WRITER, true otherwise. */ @@ -126,28 +132,27 @@ dfm_put_record (struct dfm_writer *w, const char *rec, size_t len) bool dfm_close_writer (struct dfm_writer *w) { - char *file_name; bool ok; if (w == NULL) return true; - file_name = xstrdup (fh_get_name (w->fh)); - if (fh_close (w->fh, "data file", "ws")) - { - free (file_name); - return true; - } + if (fh_unlock (w->lock)) + return true; ok = true; if (w->file != NULL) { + const char *file_name = fh_get_file_name (w->fh); ok = !dfm_write_error (w) && !fn_close (file_name, w->file); if (!ok) msg (ME, _("I/O error occurred writing data file \"%s\"."), file_name); + + if (ok ? !replace_file_commit (w->rf) : !replace_file_abort (w->rf)) + ok = false; } + fh_unref (w->fh); free (w); - free (file_name); return ok; } diff --git a/src/language/data-io/file-handle.q b/src/language/data-io/file-handle.q index 71cc961f..d5301773 100644 --- a/src/language/data-io/file-handle.q +++ b/src/language/data-io/file-handle.q @@ -133,8 +133,7 @@ cmd_close_file_handle (struct lexer *lexer, struct dataset *ds UNUSED) if (handle == NULL) return CMD_CASCADING_FAILURE; - fh_free (handle); - + fh_unname (handle); return CMD_SUCCESS; } @@ -158,7 +157,10 @@ referent_name (enum fh_referent referent) /* Parses a file handle name, which may be a file name as a string or a file handle name as an identifier. The allowed types of file handle are restricted to those in REFERENT_MASK. Returns - the file handle when successful, a null pointer on failure. */ + the file handle when successful, a null pointer on failure. + + The caller is responsible for fh_unref()'ing the returned + file handle when it is no longer needed. */ struct file_handle * fh_parse (struct lexer *lexer, enum fh_referent referent_mask) { @@ -177,8 +179,6 @@ fh_parse (struct lexer *lexer, enum fh_referent referent_mask) handle = NULL; if (lex_token (lexer) == T_ID) handle = fh_from_id (lex_tokid (lexer)); - if (handle == NULL) - handle = fh_from_file_name (ds_cstr (lex_tokstr (lexer))); if (handle == NULL) { if (lex_token (lexer) != T_ID || lex_tokid (lexer)[0] != '#' || get_syntax () != ENHANCED) @@ -194,6 +194,7 @@ fh_parse (struct lexer *lexer, enum fh_referent referent_mask) { msg (SE, _("Handle for %s not allowed here."), referent_name (fh_get_referent (handle))); + fh_unref (handle); return NULL; } diff --git a/src/language/data-io/get.c b/src/language/data-io/get.c index b861aca5..d22a4e50 100644 --- a/src/language/data-io/get.c +++ b/src/language/data-io/get.c @@ -88,6 +88,7 @@ parse_read_command (struct lexer *lexer, struct dataset *ds, enum reader_command { lex_match (lexer, '='); + fh_unref (fh); fh = fh_parse (lexer, FH_REF_FILE | FH_REF_SCRATCH); if (fh == NULL) goto error; @@ -140,9 +141,11 @@ parse_read_command (struct lexer *lexer, struct dataset *ds, enum reader_command proc_set_active_file (ds, reader, dict); + fh_unref (fh); return CMD_SUCCESS; error: + fh_unref (fh); casereader_destroy (reader); if (dict != NULL) dict_destroy (dict); @@ -368,9 +371,11 @@ parse_write_command (struct lexer *lexer, struct dataset *ds, map); dict_destroy (dict); + fh_unref (handle); return writer; error: + fh_unref (handle); casewriter_destroy (writer); dict_destroy (dict); case_map_destroy (map); @@ -1089,6 +1094,7 @@ mtf_close_all_files (struct mtf_proc *mtf) ll_for_each_preremove (file, struct mtf_file, ll, &mtf->files) { + fh_unref (file->handle); casereader_destroy (file->reader); free (file->by); dict_destroy (file->dict); diff --git a/src/language/data-io/inpt-pgm.c b/src/language/data-io/inpt-pgm.c index 844fe2fe..b8a31eae 100644 --- a/src/language/data-io/inpt-pgm.c +++ b/src/language/data-io/inpt-pgm.c @@ -296,6 +296,7 @@ cmd_reread (struct lexer *lexer, struct dataset *ds) else if (lex_match_id (lexer, "FILE")) { lex_match (lexer, '='); + fh_unref (fh); fh = fh_parse (lexer, FH_REF_FILE | FH_REF_INLINE); if (fh == NULL) { @@ -316,6 +317,7 @@ cmd_reread (struct lexer *lexer, struct dataset *ds) t->column = e; add_transformation (ds, reread_trns_proc, reread_trns_free, t); + fh_unref (fh); return CMD_SUCCESS; } diff --git a/src/language/data-io/print-space.c b/src/language/data-io/print-space.c index 39773ec0..c3a2e09a 100644 --- a/src/language/data-io/print-space.c +++ b/src/language/data-io/print-space.c @@ -82,6 +82,7 @@ cmd_print_space (struct lexer *lexer, struct dataset *ds) writer = dfm_open_writer (handle); if (writer == NULL) { + fh_unref (handle); expr_free (expr); return CMD_FAILURE; } @@ -95,6 +96,7 @@ cmd_print_space (struct lexer *lexer, struct dataset *ds) add_transformation (ds, print_space_trns_proc, print_space_trns_free, trns); + fh_unref (handle); return CMD_SUCCESS; } diff --git a/src/language/data-io/print.c b/src/language/data-io/print.c index 8ec63c4d..86d69ea9 100644 --- a/src/language/data-io/print.c +++ b/src/language/data-io/print.c @@ -204,11 +204,13 @@ internal_cmd_print (struct lexer *lexer, struct dataset *ds, add_transformation (ds, print_trns_proc, print_trns_free, trns); pool_destroy (tmp_pool); + fh_unref (fh); return CMD_SUCCESS; error: print_trns_free (trns); + fh_unref (fh); return CMD_FAILURE; } diff --git a/src/language/dictionary/ChangeLog b/src/language/dictionary/ChangeLog index fde939f9..31c762f5 100644 --- a/src/language/dictionary/ChangeLog +++ b/src/language/dictionary/ChangeLog @@ -1,3 +1,13 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * apply-dictionary.c (cmd_apply_dictionary): Manage file handle + reference counts. + + * sys-file-info.c (cmd_sysfile_info): Manage file handle reference + counts. + 2007-08-12 Ben Pfaff Output variable measurement level, alignment, and display width as diff --git a/src/language/dictionary/apply-dictionary.c b/src/language/dictionary/apply-dictionary.c index 0b724044..ff4d7853 100644 --- a/src/language/dictionary/apply-dictionary.c +++ b/src/language/dictionary/apply-dictionary.c @@ -50,13 +50,15 @@ cmd_apply_dictionary (struct lexer *lexer, struct dataset *ds) lex_match_id (lexer, "FROM"); lex_match (lexer, '='); + handle = fh_parse (lexer, FH_REF_FILE | FH_REF_SCRATCH); if (!handle) return CMD_FAILURE; - reader = any_reader_open (handle, &dict); + fh_unref (handle); if (dict == NULL) return CMD_FAILURE; + casereader_destroy (reader); for (i = 0; i < dict_get_var_cnt (dict); i++) diff --git a/src/language/dictionary/sys-file-info.c b/src/language/dictionary/sys-file-info.c index 2369b99e..fb953c94 100644 --- a/src/language/dictionary/sys-file-info.c +++ b/src/language/dictionary/sys-file-info.c @@ -99,7 +99,10 @@ cmd_sysfile_info (struct lexer *lexer, struct dataset *ds UNUSED) reader = sfm_open_reader (h, &d, &info); if (!reader) - return CMD_FAILURE; + { + fh_unref (h); + return CMD_FAILURE; + } casereader_destroy (reader); t = tab_create (2, 10, 0); @@ -184,6 +187,7 @@ cmd_sysfile_info (struct lexer *lexer, struct dataset *ds UNUSED) dict_destroy (d); + fh_unref (h); return lex_end_of_command (lexer); } diff --git a/src/language/stats/ChangeLog b/src/language/stats/ChangeLog index 146640f3..21c28c79 100644 --- a/src/language/stats/ChangeLog +++ b/src/language/stats/ChangeLog @@ -1,3 +1,16 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * aggregate.c (cmd_aggregate): Manage file handle reference + counts. + + * correlations.q (internal_cmd_frequencies): Ditto. + (cor_custom_matrix): Ditto. + + * regression.q (regression_custom_export): Ditto. + (cmd_regression): Ditto. + 2007-10-12 Ben Pfaff * flip.c (flip_file): No need to conditionally substitute for diff --git a/src/language/stats/aggregate.c b/src/language/stats/aggregate.c index c9d68d35..18d78f23 100644 --- a/src/language/stats/aggregate.c +++ b/src/language/stats/aggregate.c @@ -330,6 +330,7 @@ cmd_aggregate (struct lexer *lexer, struct dataset *ds) } agr_destroy (&agr); + fh_unref (out_file); return CMD_SUCCESS; error: @@ -337,6 +338,7 @@ error: proc_commit (ds); casewriter_destroy (output); agr_destroy (&agr); + fh_unref (out_file); return CMD_CASCADING_FAILURE; } diff --git a/src/language/stats/correlations.q b/src/language/stats/correlations.q index 540024db..5d543098 100644 --- a/src/language/stats/correlations.q +++ b/src/language/stats/correlations.q @@ -77,9 +77,13 @@ internal_cmd_correlations (struct lexer *lexer, struct dataset *ds) matrix_file = NULL; if (!parse_correlations (lexer, ds, &cmd, NULL)) - return CMD_FAILURE; + { + fh_unref (matrix_file); + return CMD_FAILURE; + } free_correlations (&cmd); + fh_unref (matrix_file); return CMD_SUCCESS; } @@ -141,6 +145,7 @@ cor_custom_matrix (struct lexer *lexer, struct dataset *ds UNUSED, struct cmd_co matrix_file = NULL; else { + fh_unref (matrix_file); matrix_file = fh_parse (lexer, FH_REF_FILE); if (matrix_file == NULL) return 0; diff --git a/src/language/stats/regression.q b/src/language/stats/regression.q index 515081a4..e3b3f5ee 100644 --- a/src/language/stats/regression.q +++ b/src/language/stats/regression.q @@ -930,6 +930,7 @@ regression_custom_export (struct lexer *lexer, struct dataset *ds UNUSED, model_file = NULL; else { + fh_unref (model_file); model_file = fh_parse (lexer, FH_REF_FILE); if (model_file == NULL) return 0; @@ -950,8 +951,12 @@ cmd_regression (struct lexer *lexer, struct dataset *ds) bool ok; size_t i; + model_file = NULL; if (!parse_regression (lexer, ds, &cmd, NULL)) - return CMD_FAILURE; + { + fh_unref (model_file); + return CMD_FAILURE; + } models = xnmalloc (cmd.n_dependent, sizeof *models); for (i = 0; i < cmd.n_dependent; i++) @@ -970,6 +975,7 @@ cmd_regression (struct lexer *lexer, struct dataset *ds) free (v_variables); free (models); free_regression (&cmd); + fh_unref (model_file); return ok ? CMD_SUCCESS : CMD_FAILURE; } diff --git a/src/ui/terminal/ChangeLog b/src/ui/terminal/ChangeLog index c626af0a..b4d329da 100644 --- a/src/ui/terminal/ChangeLog +++ b/src/ui/terminal/ChangeLog @@ -1,3 +1,12 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * main.c (main): Use at_fatal_signal instead of calling signal + directly. + (interrupt_handler) Removed. + (terminate) Rename clean_up. Don't call exit directly. + 2007-09-25 Ben Pfaff Patch #6210: implement ability to resize output device parameters diff --git a/src/ui/terminal/main.c b/src/ui/terminal/main.c index e3d3dbd7..2f80cb57 100644 --- a/src/ui/terminal/main.c +++ b/src/ui/terminal/main.c @@ -53,6 +53,7 @@ #include #include +#include "fatal-signal.h" #include "progname.h" #include "gettext.h" @@ -61,7 +62,7 @@ static void i18n_init (void); static void fpu_init (void); -static void terminate (bool success) NO_RETURN; +static void clean_up (void); /* If a segfault happens, issue a message to that effect and halt */ void bug_handler(int sig); @@ -84,7 +85,7 @@ main (int argc, char **argv) signal (SIGABRT, bug_handler); signal (SIGSEGV, bug_handler); signal (SIGFPE, bug_handler); - signal (SIGINT, interrupt_handler); + at_fatal_signal (clean_up); i18n_init (); fpu_init (); @@ -137,7 +138,8 @@ main (int argc, char **argv) } } - terminate (!any_errors ()); + clean_up (); + return any_errors (); } static void @@ -188,17 +190,9 @@ bug_handler(int sig) } } -void -interrupt_handler(int sig UNUSED) -{ - terminate (false); -} - - -/* Terminate PSPP. SUCCESS should be true to exit successfully, - false to exit as a failure. */ +/* Clean up PSPP in preparation for termination. */ static void -terminate (bool success) +clean_up (void) { static bool terminating = false; if (!terminating) @@ -214,10 +208,8 @@ terminate (bool success) destroy_source_stream (the_source_stream); prompt_done (); readln_uninitialize (); - outp_done (); msg_ui_done (); fmt_done (); } - exit (success ? EXIT_SUCCESS : EXIT_FAILURE); } diff --git a/tests/ChangeLog b/tests/ChangeLog index 555e24cf..776e7a4f 100644 --- a/tests/ChangeLog +++ b/tests/ChangeLog @@ -1,3 +1,16 @@ +2007-11-03 Ben Pfaff + + Allow output files to overwrite input files (bug #21280). + + * automake.mk: Add new file. + + * bugs/overwrite-input-file.sh: Rewrite to make sure that we can + overwrite input files safely. + + * bugs/overwrite-special-file.sh: New test. + + * command/erase.sh: Fix "activity" message. + 2007-11-03 John Darrington * Book1.gnm.unzipped command/get-data-gnm.sh: New test and data diff --git a/tests/automake.mk b/tests/automake.mk index b673bef3..dcec8da6 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -108,6 +108,7 @@ dist_TESTS = \ tests/bugs/match-files-scratch.sh \ tests/bugs/multipass.sh \ tests/bugs/overwrite-input-file.sh \ + tests/bugs/overwrite-special-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 index 56d569cb..185de8b4 100755 --- a/tests/bugs/overwrite-input-file.sh +++ b/tests/bugs/overwrite-input-file.sh @@ -1,9 +1,8 @@ #!/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. - +# This program tests that simultaneous input and output to a single +# file properly coexist, with the output atomically replacing the +# input if successful. TEMPDIR=/tmp/pspp-tst-$$ TESTFILE=$TEMPDIR/`basename $0`.sps @@ -55,33 +54,35 @@ mkdir -p $TEMPDIR cd $TEMPDIR -activity="create program 1" -cat > $TESTFILE < foo.data < $TESTFILE < $TESTFILE </dev/null 2>&1 +# PSPP should have terminated with a signal. POSIX requires that the exit +# status of a process terminated by a signal be greater than 128. +if [ $? -le 128 ] ; then no_result ; fi + +activity="check for remaining temporary files" +if test -e *.tmp*; then fail; fi + +activity="compare output 1" +cmp foo.sav foo.sav.backup +if [ $? -ne 0 ] ; then fail ; fi + +activity="compare output 2" +cmp foo.por foo.por.backup +if [ $? -ne 0 ] ; then fail ; fi + +activity="compare output 3" +cmp foo.data foo.data.backup +if [ $? -ne 0 ] ; then fail ; fi activity="create program 3" cat > $TESTFILE < $TESTFILE < $TESTFILE <