From 9aac12c2130257396c9ee4ee7860f618dcb202b0 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 2 Feb 2007 00:57:32 +0000 Subject: [PATCH] We are using a single member in struct file_handle, the "name" field, for more than one purpose. When it begins with '"', it's a file name; otherwise, it's a token that can be used to identify it. When that assertion fires, it's because we searched for the name case-sensitively as a file name (so that there was no match), and then we try to insert it case-insensitively as a token, which fails because duplicates aren't allowed. Solution: break the two purposes into two separate fields. This fixes the problem and likely makes the code easier to read too. Fixes bug #18922. Thanks to John Darrington for bug report and review. --- src/data/ChangeLog | 22 +++++++++ src/data/file-handle-def.c | 78 +++++++++++++++++++----------- src/data/file-handle-def.h | 3 +- src/language/data-io/ChangeLog | 5 ++ src/language/data-io/file-handle.q | 15 ++---- 5 files changed, 85 insertions(+), 38 deletions(-) diff --git a/src/data/ChangeLog b/src/data/ChangeLog index ac936458..5cd9a02e 100644 --- a/src/data/ChangeLog +++ b/src/data/ChangeLog @@ -1,3 +1,25 @@ +Thu Feb 1 16:53:37 2007 Ben Pfaff + + We are using a single member in struct file_handle, the "name" + field, for more than one purpose. When it begins with '"', it's a + file name; otherwise, it's a token that can be used to identify + it. When that assertion fires, it's because we searched for the + name case-sensitively as a file name (so that there was no match), + and then we try to insert it case-insensitively as a token, which + fails because duplicates aren't allowed. + + Solution: break the two purposes into two separate fields. This + fixes the problem and likely makes the code easier to read too. + + Fixes bug #18922. Thanks to John Darrington for bug report and + review. + + * file-handle-def.c (struct file_handle): New `id' member. + (fh_from_name): Rename fh_from_id. Update all callers. + (create_handle): New `id' parameter. Update all callers. + (fh_create_file): Ditto. + (fh_get_id): New function. + Mon Jan 15 16:18:10 2007 Ben Pfaff * case.c (case_is_null): Change return type to bool. diff --git a/src/data/file-handle-def.c b/src/data/file-handle-def.c index 8caccc0d..e14475b6 100644 --- a/src/data/file-handle-def.c +++ b/src/data/file-handle-def.c @@ -29,9 +29,10 @@ #include #include #include -#include "file-name.h" -#include "variable.h" -#include "scratch-handle.h" +#include +#include +#include +#include #include "gettext.h" #define _(msgid) gettext (msgid) @@ -45,7 +46,8 @@ struct file_handle int open_cnt; /* 0=not open, otherwise # of openers. */ bool deleted; /* Destroy handle when open_cnt goes to 0? */ - char *name; /* File handle identifier. */ + char id[LONG_NAME_LEN + 1]; /* Identifier token; empty string 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. */ @@ -74,13 +76,14 @@ static struct file_handle *default_handle; /* The "file" that reads from BEGIN DATA...END DATA. */ static struct file_handle *inline_file; -static struct file_handle *create_handle (const char *name, enum fh_referent); +static struct file_handle *create_handle (const char *id, + const char *name, enum fh_referent); /* File handle initialization routine. */ void fh_init (void) { - inline_file = create_handle ("INLINE", FH_REF_INLINE); + inline_file = create_handle ("INLINE", "INLINE", FH_REF_INLINE); inline_file->record_width = 80; inline_file->tab_width = 8; } @@ -116,15 +119,15 @@ fh_done (void) free_handle (file_handles); } -/* Returns the handle named HANDLE_NAME, or a null pointer if +/* Returns the handle with the given ID, or a null pointer if there is none. */ struct file_handle * -fh_from_name (const char *handle_name) +fh_from_id (const char *id) { struct file_handle *iter; for (iter = file_handles; iter != NULL; iter = iter->next) - if (!iter->deleted && !strcasecmp (handle_name, iter->name)) + if (!iter->deleted && !strcasecmp (id, iter->id)) return iter; return NULL; } @@ -164,19 +167,22 @@ fh_from_file_name (const char *file_name) return NULL; } -/* Creates a new handle with name HANDLE_NAME that refers to - REFERENT. Links the new handle into the global list. Returns - the new handle. +/* Creates a new handle with identifier ID (which may be null) + and name HANDLE_NAME that refers to REFERENT. Links the new + handle into the global list. Returns the new handle. The new handle is not fully initialized. The caller is responsible for completing its initialization. */ static struct file_handle * -create_handle (const char *handle_name, enum fh_referent referent) +create_handle (const char *id, const 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; @@ -194,16 +200,21 @@ fh_inline_file (void) return inline_file; } -/* Creates a new file handle named HANDLE_NAME, which must not be - the name of an existing file handle. The new handle is - associated with file FILE_NAME and the given PROPERTIES. */ +/* 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. */ struct file_handle * -fh_create_file (const char *handle_name, const char *file_name, +fh_create_file (const char *id, const char *file_name, const struct fh_properties *properties) { + char *handle_name; struct file_handle *handle; - assert (fh_from_name (handle_name) == NULL); - handle = create_handle (handle_name, FH_REF_FILE); + + handle_name = id != NULL ? (char *) 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; @@ -212,13 +223,15 @@ fh_create_file (const char *handle_name, const char *file_name, return handle; } -/* Creates a new file handle named HANDLE_NAME, which must not be - the name of an existing file handle. The new handle is +/* Creates a new file handle with the given ID, which must be + unique among existing file identifiers. The new handle is associated with a scratch file (initially empty). */ struct file_handle * -fh_create_scratch (const char *handle_name) +fh_create_scratch (const char *id) { - struct file_handle *handle = create_handle (handle_name, FH_REF_SCRATCH); + struct file_handle *handle; + assert (id != NULL); + handle = create_handle (id, id, FH_REF_SCRATCH); handle->sh = NULL; return handle; } @@ -368,10 +381,21 @@ fh_is_open (const struct file_handle *handle) return handle->open_cnt > 0; } -/* Returns the identifier of file HANDLE. If HANDLE was created - by referring to a file name instead of a handle name, returns - the file name, enclosed in double quotes. Return value is - owned by the file handle. +/* 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. + + Return value is owned by the file handle.*/ +const char * +fh_get_id (const struct file_handle *handle) +{ + return handle->id[0] != '\0' ? handle->id : NULL; +} + +/* Returns a user-friendly string to identify the given HANDLE. + If HANDLE was created by referring to a file name, returns the + file name, enclosed in double quotes. Return value is owned + by the file handle. Useful for printing error messages about use of file handles. */ const char * diff --git a/src/data/file-handle-def.h b/src/data/file-handle-def.h index 7c8a00a6..a3d837ab 100644 --- a/src/data/file-handle-def.h +++ b/src/data/file-handle-def.h @@ -61,11 +61,12 @@ const struct fh_properties *fh_default_properties (void); void fh_free (struct file_handle *); /* Finding file handles. */ -struct file_handle *fh_from_name (const char *handle_name); +struct file_handle *fh_from_id (const char *handle_name); struct file_handle *fh_from_file_name (const char *file_name); struct file_handle *fh_inline_file (void); /* Generic properties of file handles. */ +const char *fh_get_id (const struct file_handle *); const char *fh_get_name (const struct file_handle *); enum fh_referent fh_get_referent (const struct file_handle *); diff --git a/src/language/data-io/ChangeLog b/src/language/data-io/ChangeLog index a0a4da02..7685118d 100644 --- a/src/language/data-io/ChangeLog +++ b/src/language/data-io/ChangeLog @@ -1,3 +1,8 @@ +Thu Feb 1 16:56:02 2007 Ben Pfaff + + * file-handle.q (fh_parse): Update to new fh_create_file + prototype. + Sat Dec 16 22:16:18 2006 Ben Pfaff Make it possible to pull cases from the active file with a diff --git a/src/language/data-io/file-handle.q b/src/language/data-io/file-handle.q index 90b33cc4..6ace9128 100644 --- a/src/language/data-io/file-handle.q +++ b/src/language/data-io/file-handle.q @@ -61,7 +61,7 @@ cmd_file_handle (struct lexer *lexer, struct dataset *ds) return CMD_CASCADING_FAILURE; str_copy_trunc (handle_name, sizeof handle_name, lex_tokid (lexer)); - handle = fh_from_name (handle_name); + handle = fh_from_id (handle_name); if (handle != NULL) { msg (SE, _("File handle %s is already defined. " @@ -131,7 +131,7 @@ cmd_close_file_handle (struct lexer *lexer, struct dataset *ds UNUSED) if (!lex_force_id (lexer)) return CMD_CASCADING_FAILURE; - handle = fh_from_name (lex_tokid (lexer)); + handle = fh_from_id (lex_tokid (lexer)); if (handle == NULL) return CMD_CASCADING_FAILURE; @@ -178,19 +178,14 @@ fh_parse (struct lexer *lexer, enum fh_referent referent_mask) handle = NULL; if (lex_token (lexer) == T_ID) - handle = fh_from_name (lex_tokid (lexer)); + 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) - { - char *file_name = ds_cstr (lex_tokstr (lexer)); - char *handle_name = xasprintf ("\"%s\"", file_name); - handle = fh_create_file (handle_name, file_name, - fh_default_properties ()); - free (handle_name); - } + handle = fh_create_file (NULL, ds_cstr (lex_tokstr (lexer)), + fh_default_properties ()); else handle = fh_create_scratch (lex_tokid (lexer)); } -- 2.30.2