From 8268febfe0c336ff47a61d0d416fd4bebf61a993 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Fri, 28 Feb 2014 13:45:34 -0800 Subject: [PATCH] Fix a few crashes and leaks when cloning C strings. * alloc.c, lisp.h (dupstring): New function. * gtkutil.c (xg_get_font): * term.c (tty_default_color_capabilities): * xsettings.c (store_monospaced_changed) (store_font_name_changed, parse_settings) (read_and_apply_settings, init_gsettings, init_gconf): Use it. This avoids some unlikely crashes due to accessing freed storage, and avoids some minor memory leaks in the more-typical case. --- src/ChangeLog | 12 ++++++++++++ src/alloc.c | 14 ++++++++++++++ src/gtkutil.c | 3 +-- src/lisp.h | 1 + src/term.c | 14 +++----------- src/xsettings.c | 23 +++++++++-------------- 6 files changed, 40 insertions(+), 27 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index d2eeab33e4..704ac68b67 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,15 @@ +2014-02-28 Paul Eggert + + Fix a few crashes and leaks when cloning C strings. + * alloc.c, lisp.h (dupstring): New function. + * gtkutil.c (xg_get_font): + * term.c (tty_default_color_capabilities): + * xsettings.c (store_monospaced_changed) + (store_font_name_changed, parse_settings) + (read_and_apply_settings, init_gsettings, init_gconf): Use it. + This avoids some unlikely crashes due to accessing freed storage, + and avoids some minor memory leaks in the more-typical case. + 2014-02-28 Martin Rudalics * xdisp.c (note_mode_line_or_margin_highlight): Don't show drag diff --git a/src/alloc.c b/src/alloc.c index 7f0a74ca83..7c671e25cf 100644 --- a/src/alloc.c +++ b/src/alloc.c @@ -850,6 +850,20 @@ xlispstrdup (Lisp_Object string) return memcpy (xmalloc (size), SSDATA (string), size); } +/* Assign to *PTR a copy of STRING, freeing any storage *PTR formerly + pointed to. If STRING is null, assign it without copying anything. + Allocate before freeing, to avoid a dangling pointer if allocation + fails. */ + +void +dupstring (char **ptr, char const *string) +{ + char *old = *ptr; + *ptr = string ? xstrdup (string) : 0; + xfree (old); +} + + /* Like putenv, but (1) use the equivalent of xmalloc and (2) the argument is a const pointer. */ diff --git a/src/gtkutil.c b/src/gtkutil.c index 6e039c7a14..cebff68614 100644 --- a/src/gtkutil.c +++ b/src/gtkutil.c @@ -2106,8 +2106,7 @@ xg_get_font (struct frame *f, const char *default_name) font = Ffont_spec (8, args); pango_font_description_free (desc); - xfree (x_last_font_name); - x_last_font_name = xstrdup (name); + dupstring (&x_last_font_name, name); } #else /* Use old font selector, which just returns the font name. */ diff --git a/src/lisp.h b/src/lisp.h index a7e80a41c2..2f9a30fdfe 100644 --- a/src/lisp.h +++ b/src/lisp.h @@ -4398,6 +4398,7 @@ extern void *xpalloc (void *, ptrdiff_t *, ptrdiff_t, ptrdiff_t, ptrdiff_t); extern char *xstrdup (const char *); extern char *xlispstrdup (Lisp_Object); +extern void dupstring (char **, char const *); extern void xputenv (const char *); extern char *egetenv (const char *); diff --git a/src/term.c b/src/term.c index 0c40c2d16b..61a8d39d97 100644 --- a/src/term.c +++ b/src/term.c @@ -2052,17 +2052,9 @@ tty_default_color_capabilities (struct tty_display_info *tty, bool save) if (save) { - xfree (default_orig_pair); - default_orig_pair = tty->TS_orig_pair ? xstrdup (tty->TS_orig_pair) : NULL; - - xfree (default_set_foreground); - default_set_foreground = tty->TS_set_foreground ? xstrdup (tty->TS_set_foreground) - : NULL; - - xfree (default_set_background); - default_set_background = tty->TS_set_background ? xstrdup (tty->TS_set_background) - : NULL; - + dupstring (&default_orig_pair, tty->TS_orig_pair); + dupstring (&default_set_foreground, tty->TS_set_foreground); + dupstring (&default_set_background, tty->TS_set_background); default_max_colors = tty->TN_max_colors; default_max_pairs = tty->TN_max_pairs; default_no_color_video = tty->TN_no_color_video; diff --git a/src/xsettings.c b/src/xsettings.c index 458c3d45e9..844da19f63 100644 --- a/src/xsettings.c +++ b/src/xsettings.c @@ -91,8 +91,7 @@ store_monospaced_changed (const char *newfont) if (current_mono_font != NULL && strcmp (newfont, current_mono_font) == 0) return; /* No change. */ - xfree (current_mono_font); - current_mono_font = xstrdup (newfont); + dupstring (¤t_mono_font, newfont); if (dpyinfo_valid (first_dpyinfo) && use_system_font) { @@ -111,8 +110,7 @@ store_font_name_changed (const char *newfont) if (current_font != NULL && strcmp (newfont, current_font) == 0) return; /* No change. */ - xfree (current_font); - current_font = xstrdup (newfont); + dupstring (¤t_font, newfont); if (dpyinfo_valid (first_dpyinfo)) { @@ -492,13 +490,13 @@ parse_settings (unsigned char *prop, ++settings_seen; if (strcmp (name, XSETTINGS_TOOL_BAR_STYLE) == 0) { - settings->tb_style = xstrdup (sval); + dupstring (&settings->tb_style, sval); settings->seen |= SEEN_TB_STYLE; } #ifdef HAVE_XFT else if (strcmp (name, XSETTINGS_FONT_NAME) == 0) { - settings->font = xstrdup (sval); + dupstring (&settings->font, sval); settings->seen |= SEEN_FONT; } else if (strcmp (name, "Xft/Antialias") == 0) @@ -742,10 +740,7 @@ read_and_apply_settings (struct x_display_info *dpyinfo, int send_event_p) if (send_event_p) store_font_name_changed (settings.font); else - { - xfree (current_font); - current_font = xstrdup (settings.font); - } + dupstring (¤t_font, settings.font); xfree (settings.font); } #endif @@ -835,7 +830,7 @@ init_gsettings (void) { g_variant_ref_sink (val); if (g_variant_is_of_type (val, G_VARIANT_TYPE_STRING)) - current_mono_font = xstrdup (g_variant_get_string (val, NULL)); + dupstring (¤t_mono_font, g_variant_get_string (val, NULL)); g_variant_unref (val); } @@ -844,7 +839,7 @@ init_gsettings (void) { g_variant_ref_sink (val); if (g_variant_is_of_type (val, G_VARIANT_TYPE_STRING)) - current_font = xstrdup (g_variant_get_string (val, NULL)); + dupstring (¤t_font, g_variant_get_string (val, NULL)); g_variant_unref (val); } #endif /* HAVE_XFT */ @@ -886,13 +881,13 @@ init_gconf (void) s = gconf_client_get_string (gconf_client, GCONF_MONO_FONT, NULL); if (s) { - current_mono_font = xstrdup (s); + dupstring (¤t_mono_font, s); g_free (s); } s = gconf_client_get_string (gconf_client, GCONF_FONT_NAME, NULL); if (s) { - current_font = xstrdup (s); + dupstring (¤t_font, s); g_free (s); } gconf_client_add_dir (gconf_client, -- 2.39.2