Commit f8ad082d authored by Morten Welinder's avatar Morten Welinder

Names: fix memory corruption.

parent 74ae64e2
2014-03-02 Morten Welinder <terra@gnome.org>
* src/wbc-gtk.c (cb_workbook_debug_info): New debug flag
name-collections.
* src/expr-name.c (gnm_named_expr_collection_dump)
(gnm_named_expr_collection_sanity_check): New function.
* src/application.c (gnm_app_sanity_check): New function.
* src/expr-name.c (gnm_named_expr_collection_new): Don't use the
string inside the GOString as a hash key. It can change.
(gnm_named_expr_collection_foreach): As a consequence of the
above, the first argument to the handler for
gnm_named_expr_collection_foreach changes to something
unspecified. All callers changed.
* src/sheet.c (sheet_dup): Don't flip display-outlines for the new
sheet.
......
......@@ -51,6 +51,7 @@ Morten:
* Don't generate value format during xls read. [#725453]
* Fix name criticals.
* Fix sheet duplication issue. [#725504]
* Fix named expressions crasher. [Part of #725459]
--------------------------------------------------------------------------
Gnumeric 1.12.11
......
......@@ -6413,7 +6413,8 @@ excel_write_v8 (ExcelWriteState *ewb, GsfOutfile *outfile)
/****************************************************************************/
static void
cb_check_names (gpointer key, GnmNamedExpr *nexpr, ExcelWriteState *ewb)
cb_check_names (G_GNUC_UNUSED gconstpointer key,
GnmNamedExpr *nexpr, ExcelWriteState *ewb)
{
if (expr_name_is_active (nexpr))
excel_write_prep_expr (ewb, nexpr->texpr);
......
......@@ -1824,16 +1824,18 @@ odf_fix_en_validate (char const *name, odf_fix_expr_names_t *fen)
}
static void
odf_fix_en_collect (gchar const *key, G_GNUC_UNUSED GnmNamedExpr *nexpr, odf_fix_expr_names_t *fen)
odf_fix_en_collect (G_GNUC_UNUSED gconstpointer key_,
GnmNamedExpr *nexpr, odf_fix_expr_names_t *fen)
{
GString *str;
gchar *here;
const char *name = expr_name_name (nexpr);
if (expr_name_validate (key))
if (expr_name_validate (name))
return;
if (NULL != g_hash_table_lookup (fen->orig2fixed, key))
if (NULL != g_hash_table_lookup (fen->orig2fixed, name))
return;
str = g_string_new (key);
str = g_string_new (name);
for (here = str->str; *here; here = g_utf8_next_char (here)) {
if (!g_unichar_isalnum (g_utf8_get_char (here)) &&
......@@ -1845,13 +1847,14 @@ odf_fix_en_collect (gchar const *key, G_GNUC_UNUSED GnmNamedExpr *nexpr, odf_fix
}
while (!odf_fix_en_validate (str->str, fen))
g_string_append_c (str, '_');
odf_fix_expr_names_t_add (fen, key, g_string_free (str, FALSE));
odf_fix_expr_names_t_add (fen, name, g_string_free (str, FALSE));
}
static void
odf_fix_en_find (gchar const *key, GnmNamedExpr *nexpr, odf_fix_expr_names_t *fen)
odf_fix_en_find (G_GNUC_UNUSED gconstpointer key,
GnmNamedExpr *nexpr, odf_fix_expr_names_t *fen)
{
if (strcmp (key, fen->nexpr_name) == 0)
if (strcmp (expr_name_name (nexpr), fen->nexpr_name) == 0)
fen->nexpr = nexpr;
}
......
......@@ -25,6 +25,8 @@
#include "sheet-object.h"
#include "commands.h"
#include "gui-clipboard.h"
#include "expr-name.h"
#include "workbook-priv.h"
#include <gnumeric-conf.h>
#include <goffice/goffice.h>
......@@ -154,6 +156,22 @@ gnm_app_workbook_list (void)
return app->workbook_list;
}
void
gnm_app_sanity_check (void)
{
GList *l;
gboolean err = FALSE;
for (l = gnm_app_workbook_list (); l; l = l->next) {
Workbook *wb = l->data;
if (gnm_named_expr_collection_sanity_check (wb->names, "workbook"))
err = TRUE;
}
if (err)
g_error ("Sanity check failed\n");
}
/**
* gnm_app_clipboard_clear:
*
......
......@@ -24,6 +24,8 @@ Workbook *gnm_app_workbook_get_by_index (int i);
GSList *gnm_app_history_get_list (int max_elements);
void gnm_app_history_add (char const *filename, const char *mimetype);
void gnm_app_sanity_check (void);
void gnm_app_recalc (void);
void gnm_app_recalc_start (void);
void gnm_app_recalc_finish (void);
......
......@@ -2114,7 +2114,7 @@ struct cb_remote_names {
};
static void
cb_remote_names1 (G_GNUC_UNUSED const char *name,
cb_remote_names1 (G_GNUC_UNUSED gconstpointer key,
GnmNamedExpr *nexpr,
struct cb_remote_names *data)
{
......
......@@ -174,6 +174,33 @@ expr_name_relink_deps (GnmNamedExpr *nexpr)
g_slist_free (deps);
}
static guint
fake_go_string_hash (gconstpointer s_)
{
const GOString *s = s_;
return g_str_hash (s->str);
}
static gboolean
fake_go_string_equal (gconstpointer a_, gconstpointer b_)
{
const GOString *a = a_;
const GOString *b = b_;
return g_str_equal (a->str, b->str);
}
struct _GnmNamedExprCollection {
/* all the defined names */
GHashTable *names;
/* placeholders for references to undefined names */
GHashTable *placeholders;
/* <private> */
unsigned ref_count; /* boxed type */
};
/**
* gnm_named_expr_collection_new:
*
......@@ -184,9 +211,11 @@ gnm_named_expr_collection_new (void)
{
GnmNamedExprCollection *res = g_new (GnmNamedExprCollection, 1);
res->names = g_hash_table_new_full (g_str_hash, g_str_equal,
NULL, (GDestroyNotify) cb_nexpr_remove);
res->placeholders = g_hash_table_new_full (g_str_hash, g_str_equal,
res->names = g_hash_table_new_full
(fake_go_string_hash, fake_go_string_equal,
NULL, (GDestroyNotify) cb_nexpr_remove);
res->placeholders = g_hash_table_new_full
(fake_go_string_hash, fake_go_string_equal,
NULL, (GDestroyNotify) cb_nexpr_remove);
res->ref_count = 1;
......@@ -221,6 +250,76 @@ gnm_named_expr_collection_ref (GnmNamedExprCollection *names)
return names;
}
void
gnm_named_expr_collection_dump (GnmNamedExprCollection *names, const char *id)
{
g_printerr ("Named collection %s\n", id);
if (!names) {
g_printerr (" Empty\n");
return;
}
if (names->names && g_hash_table_size (names->names)) {
GHashTableIter hiter;
gpointer key, value;
g_printerr (" Defined names:\n");
g_hash_table_iter_init (&hiter, names->names);
while (g_hash_table_iter_next (&hiter, &key, &value)) {
const GOString *name = key;
GnmNamedExpr const *nexpr = value;
g_printerr (" [%s] =>\n", name->str);
if (name != nexpr->name)
g_printerr (" Weird keys: %p vs %p\n",
name, nexpr->name);
}
}
if (names->placeholders && g_hash_table_size (names->placeholders)) {
GHashTableIter hiter;
gpointer key, value;
g_printerr (" Defined placeholders:\n");
g_hash_table_iter_init (&hiter, names->placeholders);
while (g_hash_table_iter_next (&hiter, &key, &value)) {
const GOString *name = key;
GnmNamedExpr const *nexpr = value;
g_printerr (" [%s] =>\n", name->str);
if (name != nexpr->name)
g_printerr (" Weird keys: %p vs %p\n",
name, nexpr->name);
}
}
}
gboolean
gnm_named_expr_collection_sanity_check (GnmNamedExprCollection *names,
const char *id)
{
gboolean err = FALSE;
g_printerr ("Checking sanity for container %s\n", id);
if (names->names) {
GHashTableIter hiter;
gpointer key, value;
g_hash_table_iter_init (&hiter, names->names);
while (g_hash_table_iter_next (&hiter, &key, &value)) {
const GOString *name = key;
GnmNamedExpr const *nexpr = value;
if (name != nexpr->name) {
err = TRUE;
g_printerr ("Container %s has strange defined name\n",
id);
g_printerr (" key is %p [%s]\n",
name, name->str);
g_printerr (" target's name is %p [%s]\n",
nexpr->name, nexpr->name->str);
}
}
}
return err;
}
GType
gnm_named_expr_collection_get_type (void)
{
......@@ -280,9 +379,14 @@ gnm_named_expr_collection_lookup (GnmNamedExprCollection const *scope,
char const *name)
{
if (scope != NULL) {
GnmNamedExpr *nexpr = g_hash_table_lookup (scope->names, name);
GOString fake_name;
GnmNamedExpr *nexpr;
fake_name.str = name;
nexpr = g_hash_table_lookup (scope->names, &fake_name);
if (nexpr == NULL)
nexpr = g_hash_table_lookup (scope->placeholders, name);
nexpr = g_hash_table_lookup (scope->placeholders,
&fake_name);
return nexpr;
} else
return NULL;
......@@ -333,8 +437,9 @@ gnm_named_expr_collection_insert (GnmNamedExprCollection *scope,
/* name can be active at this point, eg we are converting a
* placeholder, or changing a scope */
nexpr->scope = scope;
g_hash_table_replace (nexpr->is_placeholder
? scope->placeholders : scope->names, (gpointer)nexpr->name->str, nexpr);
g_hash_table_replace
(nexpr->is_placeholder ? scope->placeholders : scope->names,
(gpointer)nexpr->name, nexpr);
}
typedef struct {
......@@ -620,12 +725,15 @@ expr_name_add (GnmParsePos const *pp, char const *name,
{
GnmNamedExpr *nexpr = NULL;
GnmNamedExprCollection *scope = NULL;
GOString fake_name;
g_return_val_if_fail (pp != NULL, NULL);
g_return_val_if_fail (pp->sheet != NULL || pp->wb != NULL, NULL);
g_return_val_if_fail (name != NULL, NULL);
g_return_val_if_fail (stub == NULL || stub->is_placeholder, NULL);
fake_name.str = name;
if (texpr != NULL && expr_name_check_for_loop (name, texpr)) {
gnm_expr_top_unref (texpr);
if (error_msg)
......@@ -635,7 +743,7 @@ expr_name_add (GnmParsePos const *pp, char const *name,
scope = (pp->sheet != NULL) ? pp->sheet->names : pp->wb->names;
/* see if there was a place holder */
nexpr = g_hash_table_lookup (scope->placeholders, name);
nexpr = g_hash_table_lookup (scope->placeholders, &fake_name);
if (nexpr != NULL) {
if (texpr == NULL) {
/* there was already a placeholder for this */
......@@ -644,10 +752,10 @@ expr_name_add (GnmParsePos const *pp, char const *name,
}
/* convert the placeholder into a real name */
g_hash_table_steal (scope->placeholders, name);
g_hash_table_steal (scope->placeholders, &fake_name);
nexpr->is_placeholder = FALSE;
} else {
nexpr = g_hash_table_lookup (scope->names, name);
nexpr = g_hash_table_lookup (scope->names, &fake_name);
/* If this is a permanent name, we may be adding it */
/* on opening of a file, although */
/* the name is already in place. */
......@@ -767,7 +875,7 @@ expr_name_remove (GnmNamedExpr *nexpr)
g_hash_table_remove (
nexpr->is_placeholder ? nexpr->scope->placeholders : nexpr->scope->names,
nexpr->name->str);
nexpr->name);
}
const char *
......@@ -790,6 +898,7 @@ expr_name_set_name (GnmNamedExpr *nexpr,
{
const char *old_name;
GHashTable *h;
GOString fake_new_name;
g_return_val_if_fail (nexpr != NULL, TRUE);
g_return_val_if_fail (nexpr->scope == NULL || new_name, TRUE);
......@@ -798,6 +907,7 @@ expr_name_set_name (GnmNamedExpr *nexpr,
if (go_str_compare (new_name, old_name) == 0)
return FALSE;
fake_new_name.str = new_name;
#if 0
g_printerr ("Renaming %s to %s\n", old_name, new_name);
#endif
......@@ -808,21 +918,23 @@ expr_name_set_name (GnmNamedExpr *nexpr,
: NULL;
if (h) {
if (new_name &&
(g_hash_table_lookup (nexpr->scope->placeholders, new_name) ||
g_hash_table_lookup (nexpr->scope->names, new_name))) {
(g_hash_table_lookup (nexpr->scope->placeholders,
&fake_new_name) ||
g_hash_table_lookup (nexpr->scope->names,
&fake_new_name))) {
/* The only error not to be blamed on the programmer is
already-in-use. */
return TRUE;
}
g_hash_table_steal (h, old_name);
g_hash_table_steal (h, nexpr->name);
}
go_string_unref (nexpr->name);
nexpr->name = go_string_new (new_name);
if (h)
g_hash_table_insert (h, (gpointer)nexpr->name->str, nexpr);
g_hash_table_insert (h, (gpointer)nexpr->name, nexpr);
return FALSE;
}
......@@ -889,7 +1001,6 @@ char *
expr_name_set_pos (GnmNamedExpr *nexpr, GnmParsePos const *pp)
{
GnmNamedExprCollection *old_scope, *new_scope;
const char *name;
g_return_val_if_fail (nexpr != NULL, NULL);
g_return_val_if_fail (pp != NULL, NULL);
......@@ -897,20 +1008,19 @@ expr_name_set_pos (GnmNamedExpr *nexpr, GnmParsePos const *pp)
old_scope = nexpr->scope;
new_scope = pp->sheet ? pp->sheet->names : pp->wb->names;
name = nexpr->name->str;
if (old_scope != new_scope &&
(g_hash_table_lookup (new_scope->placeholders, name) ||
g_hash_table_lookup (new_scope->names, name))) {
(g_hash_table_lookup (new_scope->placeholders, nexpr->name) ||
g_hash_table_lookup (new_scope->names, nexpr->name))) {
const char *fmt = pp->sheet
? _("'%s' is already defined in sheet")
: _("'%s' is already defined in workbook");
return g_strdup_printf (fmt, name);
return g_strdup_printf (fmt, nexpr->name);
}
if (old_scope)
g_hash_table_steal
(nexpr->is_placeholder ? old_scope->placeholders : old_scope->names,
name);
nexpr->name);
nexpr->pos = *pp;
gnm_named_expr_collection_insert (new_scope, nexpr);
......@@ -1013,12 +1123,10 @@ expr_name_set_is_placeholder (GnmNamedExpr *nexpr, gboolean is_placeholder)
nexpr->is_placeholder = is_placeholder;
if (nexpr->scope) {
const char *name = expr_name_name (nexpr);
g_hash_table_steal (is_placeholder
? nexpr->scope->names
: nexpr->scope->placeholders,
name);
nexpr->name);
gnm_named_expr_collection_insert (nexpr->scope, nexpr);
}
}
......@@ -1036,7 +1144,7 @@ struct cb_expr_name_in_use {
};
static void
cb_expr_name_in_use (G_GNUC_UNUSED const char *name,
cb_expr_name_in_use (G_GNUC_UNUSED gconstpointer key,
GnmNamedExpr *nexpr,
struct cb_expr_name_in_use *pdata)
{
......
......@@ -65,17 +65,6 @@ GOUndo *expr_name_set_expr_undo_new (GnmNamedExpr *nexpr);
/******************************************************************************/
struct _GnmNamedExprCollection {
/* all the defined names */
GHashTable *names;
/* placeholders for references to undefined names */
GHashTable *placeholders;
/* <private> */
unsigned ref_count; /* boxed type */
};
GType gnm_named_expr_collection_get_type (void);
GnmNamedExprCollection *gnm_named_expr_collection_new (void);
void gnm_named_expr_collection_free (GnmNamedExprCollection *names);
......@@ -88,6 +77,11 @@ GSList *gnm_named_expr_collection_list (GnmNamedExprCollection const *scope);
GnmNamedExpr *gnm_named_expr_collection_lookup (GnmNamedExprCollection const *scope,
char const *name);
void gnm_named_expr_collection_dump (GnmNamedExprCollection *names,
const char *id);
gboolean gnm_named_expr_collection_sanity_check (GnmNamedExprCollection *names,
const char *id);
G_END_DECLS
#endif /* _GNM_EXPR_NAME_H_ */
......@@ -335,7 +335,9 @@ suggest_size (GSList *wbs, int *csuggest, int *rsuggest)
}
static void
cb_fixup_name_wb (const char *name, GnmNamedExpr *nexpr, Workbook *wb)
cb_fixup_name_wb (G_GNUC_UNUSED gconstpointer key,
GnmNamedExpr *nexpr,
Workbook *wb)
{
GnmParsePos newpos = nexpr->pos;
......
......@@ -20,6 +20,7 @@
#include "workbook.h"
#include "sheet.h"
#include "cell.h"
#include "expr-name.h"
#include "value.h"
#include "mstyle.h"
#include "sheet-style.h"
......@@ -150,9 +151,11 @@ ssindex_chart (IndexerState *state, GogObject *obj)
}
static void
cb_index_name (const char *name, GnmExprName const *nexpr, IndexerState *state)
cb_index_name (G_GNUC_UNUSED gconstpointer key,
GnmNamedExpr const *nexpr, IndexerState *state)
{
gsf_xml_out_simple_element (state->output, "data", name);
gsf_xml_out_simple_element (state->output,
"data", expr_name_name (nexpr));
}
......
......@@ -60,6 +60,7 @@
#include "tools/analysis-auto-expression.h"
#include "sheet-object-cell-comment.h"
#include "print-info.h"
#include "expr-name.h"
#include <goffice/goffice.h>
#include <gsf/gsf-impl-utils.h>
......@@ -2202,6 +2203,7 @@ cb_statusbox_focus (GtkEntry *entry, GdkEventFocus *event,
}
/******************************************************************************/
static void
cb_workbook_debug_info (WBCGtk *wbcg)
{
......@@ -2223,6 +2225,14 @@ cb_workbook_debug_info (WBCGtk *wbcg)
if (gnm_debug_flag ("style-optimize")) {
workbook_optimize_style (wb);
}
if (gnm_debug_flag ("name-collections")) {
gnm_named_expr_collection_dump (wb->names, "workbook");
WORKBOOK_FOREACH_SHEET(wb, sheet, {
gnm_named_expr_collection_dump (sheet->names,
sheet->name_unquoted);
});
}
}
static void
......@@ -2821,7 +2831,8 @@ wbc_gtk_create_edit_area (WBCGtk *wbcg)
debug_button = GET_GUI_ITEM ("debug_button");
if (gnm_debug_flag ("deps") ||
gnm_debug_flag ("expr-sharer") ||
gnm_debug_flag ("style-optimize")) {
gnm_debug_flag ("style-optimize") ||
gnm_debug_flag ("name-collections")) {
g_signal_connect_swapped (debug_button,
"clicked", G_CALLBACK (cb_workbook_debug_info),
wbcg);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment