From 3d8c1e50fcc2c7e2ab43e8d09bf00e0610d69eaa Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Sat, 10 Nov 2012 15:32:06 +0100 Subject: [PATCH] Fix default filename handling errors The default filename handling is broken in two different ways: (a) if we start subsurface with a non-existing file, we warn about the inability to read that file, and then we exit without setting the default filename. This is broken because it means that if the user (perhaps by mistake, by pressing ^S) now saves the file, he will overwrite the default filename, even though that was *not* the file we read, and *not* the file that subsurface was started with. So just set the default filename even for a failed file open. The exact same logic is true of a failed parse of an XML file that we successfully opened. We do *not* want to leave the old default filename in place just because the XML parsing failed, and possibly then overwriting some file that was never involved with that failure in the first place. So just get rid of all the logic to push the filename saving into the XML parsing layer, it has zero relevance at that point. (b) if we do replace the default filename with a NULL file, we need to set that even if we cannot do a strdup() on the NULL. This fixes both errors. Signed-off-by: Linus Torvalds Signed-off-by: Dirk Hohndel --- dive.h | 3 +-- file.c | 20 +++++++++++++++----- gtk-gui.c | 4 +++- parse-xml.c | 6 +----- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/dive.h b/dive.h index 949be20d3..d303c81b7 100644 --- a/dive.h +++ b/dive.h @@ -352,8 +352,7 @@ static inline struct dive *get_dive(int nr) for ((_i) = 0; ((_x) = get_dive(_i)) != NULL; (_i)++) extern void parse_xml_init(void); -extern void parse_xml_buffer(const char *url, const char *buf, int size, GError **error, - gboolean possible_default_filename); +extern void parse_xml_buffer(const char *url, const char *buf, int size, GError **error); extern void parse_xml_exit(void); extern void set_filename(const char *filename, gboolean force); diff --git a/file.c b/file.c index 93abb8b9e..db72b5f66 100644 --- a/file.c +++ b/file.c @@ -72,7 +72,7 @@ static void suunto_read(struct zip_file *file, GError **error) size = read * 3 / 2; mem = realloc(mem, size); } - parse_xml_buffer(_("SDE file"), mem, read, error, FALSE); + parse_xml_buffer(_("SDE file"), mem, read, error); free(mem); } #endif @@ -240,14 +240,13 @@ static int open_by_filename(const char *filename, const char *fmt, struct memblo return 0; } -static void parse_file_buffer(const char *filename, struct memblock *mem, GError **error, - gboolean possible_default_filename) +static void parse_file_buffer(const char *filename, struct memblock *mem, GError **error) { char *fmt = strrchr(filename, '.'); if (fmt && open_by_filename(filename, fmt+1, mem, error)) return; - parse_xml_buffer(filename, mem->buffer, mem->size, error, possible_default_filename); + parse_xml_buffer(filename, mem->buffer, mem->size, error); } void parse_file(const char *filename, GError **error, gboolean possible_default_filename) @@ -266,9 +265,20 @@ void parse_file(const char *filename, GError **error, gboolean possible_default_ _("Failed to read '%s'"), filename); } + + /* + * We do *not* want to leave the old default_filename + * just because the open failed. + */ + if (possible_default_filename) + set_filename(filename, TRUE); + return; } - parse_file_buffer(filename, &mem, error, possible_default_filename); + if (possible_default_filename) + set_filename(filename, TRUE); + + parse_file_buffer(filename, &mem, error); free(mem.buffer); } diff --git a/gtk-gui.c b/gtk-gui.c index 690b2a488..0b297b753 100644 --- a/gtk-gui.c +++ b/gtk-gui.c @@ -1518,7 +1518,7 @@ static GError *setup_uemis_import(device_data_t *data) #ifdef DEBUGFILE fprintf(debugfile, "xml buffer \"%s\"\n\n", buf); #endif - parse_xml_buffer("Uemis Download", buf, strlen(buf), &error, FALSE); + parse_xml_buffer("Uemis Download", buf, strlen(buf), &error); set_uemis_last_dive(uemis_max_dive_data); #if UEMIS_DEBUG fprintf(debugfile, "uemis_max_dive_data: %s\n", uemis_max_dive_data); @@ -1672,4 +1672,6 @@ void set_filename(const char *filename, gboolean force) free((void *)existing_filename); if (filename) existing_filename = strdup(filename); + else + existing_filename = NULL; } diff --git a/parse-xml.c b/parse-xml.c index 272b82c0f..b4fc9e716 100644 --- a/parse-xml.c +++ b/parse-xml.c @@ -1485,8 +1485,7 @@ static void reset_all(void) import_source = UNKNOWN; } -void parse_xml_buffer(const char *url, const char *buffer, int size, GError **error, - gboolean possible_default_filename) +void parse_xml_buffer(const char *url, const char *buffer, int size, GError **error) { xmlDoc *doc; @@ -1502,9 +1501,6 @@ void parse_xml_buffer(const char *url, const char *buffer, int size, GError **er } return; } - /* remember, if necessary, that this is the filename to store to */ - if (possible_default_filename) - set_filename(url, FALSE); reset_all(); dive_start(); #ifdef XSLT