This is a low-priority cleanup that may be objectionable, so I'll post here first.
According to the g_free documentation, g_free accepts a NULL argument (as a no-op), yet much code explicitly checks for NULL before calling g_free.
Disadvantage of the transformation:
- Involves a function call even in the NULL case (unless g_free is a macro or inline function that tests for NULL before passing to the "real" version). I've left a couple of places untransformed where it looks like NULL has a significant probability. If NULL is unlikely, then we're better off having more readable code [and saving a couple of bytes of object code].
- May make diff3 against sodipodi code harder.
Strong candidates for applying the transformation are where the tested pointer is something long like `style->text->font_family.value', and src/module.c where it uses a IF_NOT_NULL_FREE macro.
Btw, some (but not all) of the searching for this pattern was using a regexp to reduce the probability of accidentally transforming `if (foo) g_free (bar)' for distinct `foo' and `bar'.
Any comments on where this transformation (removing the `if') should be applied? In absence of comments, I'll probably commit the patch in its current form (other than ChangeLog date).
pjrm.
Index: ChangeLog =================================================================== RCS file: /cvsroot/inkscape/inkscape/ChangeLog,v retrieving revision 1.56 diff -d -p -U5 -r1.56 ChangeLog --- ChangeLog 29 Nov 2003 09:20:28 -0000 1.56 +++ ChangeLog 30 Nov 2003 03:34:38 -0000 @@ -7,10 +7,33 @@ Make utest.c,utest.h formatting conform more to doc/coding-style.txt.
Have utest.c built into libutest.a (see src/utest/Makefile.am comments).
+ * src/document.c (sp_document_new), file.c (file_open_ok, + sp_file_open_dialog, sp_file_save_dialog, sp_file_do_import), + main.c (sp_main_console), src/module.c (sp_module_finalize, + sp_module_input_finalize, sp_module_input_build, + sp_module_output_finalize, sp_module_output_build), + src/sp-anchor.c (sp_anchor_set), src/sp-image.c (sp_image_set), + src/sp-text.c (sp_string_release, sp_string_read_content, + sp_string_set_shape), src/sp-use.c (sp_use_release), src/style.c + (sp_style_unref, sp_style_merge_property, + sp_style_merge_from_parent, sp_text_style_unref, + sp_style_read_istring), src/bonobo/embeddable-document.c + (sp_bonobo_stream_read, sp_embeddable_document_ps_save), + src/dialogs/stroke-style.c (sp_stroke_style_scale_line), + src/dialogs/text-edit.c (sp_text_edit_dialog_text_changed), + src/helper/sodipodi-ctrl.c (sp_ctrl_build_cache), + src/modules/ps.c (sp_ps_print_image), + src/widgets/font-selector.c (sp_font_preview_set_phrase), + src/widgets/gradient-image.c (sp_gradient_image_size_allocate), + src/xml/repr-action.c (coalesce_chgattr, coalesce_chgcontent, + free_action), src/xml/repr.c (repr_finalize, sp_repr_merge): + Minor cleanup: replace `if (POINTER_EXPR) g_free (POINTER_EXPR)' + with the simpler (more readable) `g_free (POINTER_EXPR)'. + * src/xml/repr-action.c (reverse_log): Add doc comment. Rename some vars.
* src/xml/repr-action.c (sp_repr_replay_log, sp_repr_undo_log), src/xml/repr-action.h, src/document-undo.c (sp_document_cancel, sp_document_redo, sp_document_undo): Remove the unused doc Index: src/document.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/document.c,v retrieving revision 1.9 diff -d -p -U5 -r1.9 document.c --- src/document.c 29 Nov 2003 21:55:57 -0000 1.9 +++ src/document.c 30 Nov 2003 03:34:38 -0000 @@ -334,12 +334,12 @@ sp_document_new (const gchar *uri, unsig name = g_strdup_printf (_("New document %d"), ++doc_count); }
doc = sp_document_create (rdoc, uri, base, name, advertize, keepalive);
- if (base) g_free (base); - if (name) g_free (name); + g_free (base); + g_free (name);
return doc; }
SPDocument * Index: src/file.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/file.c,v retrieving revision 1.10 diff -d -p -U5 -r1.10 file.c --- src/file.c 30 Nov 2003 03:29:01 -0000 1.10 +++ src/file.c 30 Nov 2003 03:34:38 -0000 @@ -99,11 +99,11 @@ file_open_ok (GtkWidget *widget, GtkFile gchar *filename;
filename = g_strdup (gtk_file_selection_get_filename (fs));
if (filename && g_file_test (filename, G_FILE_TEST_IS_DIR)) { - if (open_path) g_free (open_path); + g_free (open_path); if (filename[strlen(filename) - 1] != G_DIR_SEPARATOR) { open_path = g_strconcat (filename, G_DIR_SEPARATOR_S, NULL); g_free (filename); } else { open_path = filename; @@ -112,11 +112,11 @@ file_open_ok (GtkWidget *widget, GtkFile return; }
if (filename != NULL) { gchar* key; - if (open_path) g_free (open_path); + g_free (open_path); open_path = g_dirname (filename); if (open_path) open_path = g_strconcat (open_path, G_DIR_SEPARATOR_S, NULL); key = (gchar*)g_object_get_data (G_OBJECT (fs), "type-key"); sp_file_open (filename, key); g_free (filename); @@ -142,11 +142,11 @@ sp_file_open_dialog (gpointer object, gp { #ifdef WIN32 char *filename; filename = sp_win32_get_open_filename (open_path, "SVG files\0*.svg;*.svgz\0All files\0*\0", _("Select file to open")); if (filename) { - if (open_path) g_free (open_path); + g_free (open_path); open_path = g_dirname (filename); if (open_path) open_path = g_strconcat (open_path, G_DIR_SEPARATOR_S, NULL); sp_file_open (filename, NULL); g_free (filename); } @@ -199,11 +199,11 @@ sp_file_save_dialog (SPDocument *doc) char *filename; unsigned int spns; filename = sp_win32_get_save_filename (save_path, &spns); if (filename && *filename) { sp_file_do_save (doc, filename, (spns) ? SP_MODULE_KEY_OUTPUT_SVG_INKSCAPE : SP_MODULE_KEY_OUTPUT_SVG); - if (save_path) g_free (save_path); + g_free (save_path); save_path = g_dirname (filename); save_path = g_strdup (save_path); g_free (filename); } #else @@ -320,11 +320,11 @@ sp_file_do_import (SPDocument *doc, cons const gchar *e, *docbase, *relname; SPRepr * repr; SPReprDoc * rnewdoc;
if (filename && g_file_test (filename, G_FILE_TEST_IS_DIR)) { - if (import_path) g_free (import_path); + g_free (import_path); if (filename[strlen(filename) - 1] != G_DIR_SEPARATOR) { import_path = g_strconcat (filename, G_DIR_SEPARATOR_S, NULL); } else { import_path = g_strdup (filename); } Index: src/main.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/main.c,v retrieving revision 1.9 diff -d -p -U5 -r1.9 main.c --- src/main.c 27 Nov 2003 06:14:27 -0000 1.9 +++ src/main.c 30 Nov 2003 03:34:38 -0000 @@ -372,11 +372,11 @@ sp_main_console (int argc, const char ** } } fl = g_slist_remove (fl, fl->data); }
- if (printer) g_free (printer); + g_free (printer);
inkscape_unref ();
return 0; } Index: src/module.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/module.c,v retrieving revision 1.5 diff -d -p -U5 -r1.5 module.c --- src/module.c 16 Nov 2003 07:09:42 -0000 1.5 +++ src/module.c 30 Nov 2003 03:34:38 -0000 @@ -27,20 +27,10 @@ #include "document.h" #include "module.h"
#include "modules/db.h"
-/** - \def IF_NOT_NULL_FREE - - This is a quick little macro to check if a value is null, and - if it is not, then free the data. It's used in most of the - finalize functions, and then also when updating values. It - doesn't do that much, but it makes the code easier to read -*/ -#define IF_NOT_NULL_FREE(x) if ((x) != NULL) g_free((x)) - /* SPModule */
static void sp_module_class_init (SPModuleClass *klass); static void sp_module_init (SPModule *module); static void sp_module_finalize (GObject *object); @@ -52,11 +42,11 @@ static SPModule * sp_module_new (GType t
/** \var module_parent_class
This is the parent class for the modules. It should be - GObject - but no promises */ + GObject - but no promises */ static GObjectClass * module_parent_class;
/** \return The type value for a SP Module \brief A quick way to get the type value for a SPModule @@ -157,12 +147,12 @@ sp_module_finalize (GObject *object)
sp_module_db_unregister (module);
if (module->repr) sp_repr_unref (module->repr);
- IF_NOT_NULL_FREE(module->name); - IF_NOT_NULL_FREE(module->id); + g_free (module->name); + g_free (module->id);
G_OBJECT_CLASS (module_parent_class)->finalize (object);
return; } @@ -285,11 +275,11 @@ sp_module_unload_default (SPModule * mod should be created. \param repr The XML structure of the module definition.
This function is used to build an SPModule (really a subclass of that though) from an XML description stored in a SPRepr tree. If the class - has it's own build function, then that function is used. Otherwise + has its own build function, then that function is used. Otherwise the generic 'sp_module_private_build' function is used.
A reference to the SPRepr structure is added in this function so it should not need to be added by each individual subclass. */ @@ -424,14 +414,14 @@ sp_module_input_finalize (GObject *objec { SPModuleInput *imod;
imod = (SPModuleInput *) object; - IF_NOT_NULL_FREE(imod->mimetype); - IF_NOT_NULL_FREE(imod->extension); - IF_NOT_NULL_FREE(imod->filetypename); - IF_NOT_NULL_FREE(imod->filetypetooltip); + g_free (imod->mimetype); + g_free (imod->extension); + g_free (imod->filetypename); + g_free (imod->filetypetooltip);
G_OBJECT_CLASS (input_parent_class)->finalize (object);
return; } @@ -470,23 +460,23 @@ sp_module_input_build (SPModule *module, while (child_repr != NULL) { if (!strcmp(sp_repr_name(child_repr), "input")) { child_repr = sp_repr_children(child_repr); while (child_repr != NULL) { if (!strcmp(sp_repr_name(child_repr), "extension")) { - IF_NOT_NULL_FREE(imod->extension); + g_free (imod->extension); imod->extension = g_strdup(sp_repr_content(sp_repr_children(child_repr))); } if (!strcmp(sp_repr_name(child_repr), "mimetype")) { - IF_NOT_NULL_FREE(imod->mimetype); + g_free (imod->mimetype); imod->mimetype = g_strdup(sp_repr_content(sp_repr_children(child_repr))); } if (!strcmp(sp_repr_name(child_repr), "filetypename")) { - IF_NOT_NULL_FREE(imod->filetypename); + g_free (imod->filetypename); imod->filetypename = g_strdup(sp_repr_content(sp_repr_children(child_repr))); } if (!strcmp(sp_repr_name(child_repr), "filetypetooltip")) { - IF_NOT_NULL_FREE(imod->filetypetooltip); + g_free (imod->filetypetooltip); imod->filetypetooltip = g_strdup(sp_repr_content(sp_repr_children(child_repr))); }
child_repr = sp_repr_next(child_repr); } @@ -623,14 +613,14 @@ sp_module_output_finalize (GObject *obje { SPModuleOutput *omod;
omod = (SPModuleOutput *) object;
- IF_NOT_NULL_FREE(omod->mimetype); - IF_NOT_NULL_FREE(omod->extension); - IF_NOT_NULL_FREE(omod->filetypename); - IF_NOT_NULL_FREE(omod->filetypetooltip); + g_free (omod->mimetype); + g_free (omod->extension); + g_free (omod->filetypename); + g_free (omod->filetypetooltip); G_OBJECT_CLASS (output_parent_class)->finalize (object); }
/** @@ -667,23 +657,23 @@ sp_module_output_build (SPModule *module while (child_repr != NULL) { if (!strcmp(sp_repr_name(child_repr), "output")) { child_repr = sp_repr_children(child_repr); while (child_repr != NULL) { if (!strcmp(sp_repr_name(child_repr), "extension")) { - IF_NOT_NULL_FREE(omod->extension); + g_free (omod->extension); omod->extension = g_strdup(sp_repr_content(sp_repr_children(child_repr))); } if (!strcmp(sp_repr_name(child_repr), "mimetype")) { - IF_NOT_NULL_FREE(omod->mimetype); + g_free (omod->mimetype); omod->mimetype = g_strdup(sp_repr_content(sp_repr_children(child_repr))); } if (!strcmp(sp_repr_name(child_repr), "filetypename")) { - IF_NOT_NULL_FREE(omod->filetypename); + g_free (omod->filetypename); omod->filetypename = g_strdup(sp_repr_content(sp_repr_children(child_repr))); } if (!strcmp(sp_repr_name(child_repr), "filetypetooltip")) { - IF_NOT_NULL_FREE(omod->filetypetooltip); + g_free (omod->filetypetooltip); omod->filetypetooltip = g_strdup(sp_repr_content(sp_repr_children(child_repr))); }
child_repr = sp_repr_next(child_repr); } Index: src/sp-anchor.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/sp-anchor.c,v retrieving revision 1.2 diff -d -p -U5 -r1.2 sp-anchor.c --- src/sp-anchor.c 2 Nov 2003 00:46:23 -0000 1.2 +++ src/sp-anchor.c 30 Nov 2003 03:34:38 -0000 @@ -130,11 +130,11 @@ sp_anchor_set (SPObject *object, unsigne
anchor = SP_ANCHOR (object);
switch (key) { case SP_ATTR_XLINK_HREF: - if (anchor->href) g_free (anchor->href); + g_free (anchor->href); anchor->href = g_strdup (value); sp_object_request_modified (object, SP_OBJECT_MODIFIED_FLAG); break; case SP_ATTR_XLINK_TYPE: case SP_ATTR_XLINK_ROLE: Index: src/sp-image.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/sp-image.c,v retrieving revision 1.2 diff -d -p -U5 -r1.2 sp-image.c --- src/sp-image.c 31 Oct 2003 07:05:50 -0000 1.2 +++ src/sp-image.c 30 Nov 2003 03:34:38 -0000 @@ -171,11 +171,11 @@ sp_image_set (SPObject *object, unsigned
image = SP_IMAGE (object);
switch (key) { case SP_ATTR_XLINK_HREF: - if (image->href) g_free (image->href); + g_free (image->href); image->href = (value) ? g_strdup (value) : NULL; sp_object_request_update (object, SP_OBJECT_MODIFIED_FLAG | SP_IMAGE_HREF_MODIFIED_FLAG); break; case SP_ATTR_X: if (sp_svg_length_read_lff (value, &unit, &image->x.value, &image->x.computed) && Index: src/sp-text.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/sp-text.c,v retrieving revision 1.6 diff -d -p -U5 -r1.6 sp-text.c --- src/sp-text.c 16 Nov 2003 23:37:19 -0000 1.6 +++ src/sp-text.c 30 Nov 2003 03:34:39 -0000 @@ -132,12 +132,12 @@ sp_string_release (SPObject *object) { SPString *string;
string = SP_STRING (object);
- if (string->p) g_free (string->p); - if (string->text) g_free (string->text); + g_free (string->p); + g_free (string->text);
if (((SPObjectClass *) string_parent_class)->release) ((SPObjectClass *) string_parent_class)->release (object); }
@@ -149,13 +149,13 @@ sp_string_read_content (SPObject *object SPString *string; const gchar *t;
string = SP_STRING (object);
- if (string->p) g_free (string->p); + g_free (string->p); string->p = NULL; - if (string->text) g_free (string->text); + g_free (string->text); t = sp_repr_content (object->repr); string->text = (t) ? g_strdup (t) : NULL;
/* Is this correct? I think so (Lauris) */ /* Virtual method will be invoked BEFORE signal, so we can update there */ @@ -295,11 +295,11 @@ sp_string_set_shape (SPString *string, S sp_chars_clear (chars);
if (!string->text || !*string->text) return; len = g_utf8_strlen (string->text, -1); if (!len) return; - if (string->p) g_free (string->p); + g_free (string->p); string->p = g_new (NRPointF, len + 1);
/* fixme: Adjusted value (Lauris) */ size = style->font_size.computed; face = nr_type_directory_lookup_fuzzy (style->text->font_family.value, sp_text_font_style_to_lookup (style)); Index: src/sp-use.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/sp-use.c,v retrieving revision 1.2 diff -d -p -U5 -r1.2 sp-use.c --- src/sp-use.c 2 Nov 2003 00:30:53 -0000 1.2 +++ src/sp-use.c 30 Nov 2003 03:34:39 -0000 @@ -147,11 +147,11 @@ sp_use_release (SPObject *object)
if (use->child) { use->child = sp_object_detach_unref (SP_OBJECT (object), use->child); }
- if (use->href) g_free (use->href); + g_free (use->href);
if (((SPObjectClass *) parent_class)->release) ((SPObjectClass *) parent_class)->release (object); }
Index: src/style.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/style.c,v retrieving revision 1.4 diff -d -p -U5 -r1.4 style.c --- src/style.c 16 Nov 2003 23:37:19 -0000 1.4 +++ src/style.c 30 Nov 2003 03:34:39 -0000 @@ -259,13 +259,11 @@ sp_style_unref (SPStyle *style) /* if (style->object) gtk_signal_disconnect_by_data (GTK_OBJECT (style->object), style); */ if (style->object) g_signal_handlers_disconnect_matched (G_OBJECT(style->object), G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, style); if (style->text) sp_text_style_unref (style->text); sp_style_paint_clear (style, &style->fill, TRUE, FALSE); sp_style_paint_clear (style, &style->stroke, TRUE, FALSE); - if (style->stroke_dash.dash) { - g_free (style->stroke_dash.dash); - } + g_free (style->stroke_dash.dash); g_free (style); }
return NULL; } @@ -472,11 +470,11 @@ sp_style_merge_property (SPStyle *style, SPS_READ_IENUM_IF_UNSET (&style->font_stretch, val, enum_font_stretch, TRUE); break; case SP_PROP_FONT: if (!style->text_private) sp_style_privatize_text (style); if (!style->text->font.set) { - if (style->text->font.value) g_free (style->text->font.value); + g_free (style->text->font.value); style->text->font.value = g_strdup (val); style->text->font.set = TRUE; style->text->font.inherit = (val && !strcmp (val, "inherit")); } break; @@ -812,11 +810,11 @@ sp_style_merge_from_parent (SPStyle *sty style->writing_mode.computed = parent->writing_mode.computed; }
if (style->text && parent->text) { if (!style->text->font_family.set || style->text->font_family.inherit) { - if (style->text->font_family.value) g_free (style->text->font_family.value); + g_free (style->text->font_family.value); style->text->font_family.value = g_strdup (parent->text->font_family.value); } } }
@@ -1222,12 +1220,12 @@ static SPTextStyle * sp_text_style_unref (SPTextStyle *st) { st->refcount -= 1;
if (st->refcount < 1) { - if (st->font.value) g_free (st->font.value); - if (st->font_family.value) g_free (st->font_family.value); + g_free (st->font.value); + g_free (st->font_family.value); g_free (st); }
return NULL; } @@ -1316,11 +1314,11 @@ sp_style_read_ienum (SPIEnum *val, const }
static void sp_style_read_istring (SPIString *val, const gchar *str) { - if (val->value) g_free (val->value); + g_free (val->value);
if (!strcmp (str, "inherit")) { val->set = TRUE; val->inherit = TRUE; val->value = NULL; Index: src/bonobo/embeddable-document.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/bonobo/embeddable-document.c,v retrieving revision 1.2 diff -d -p -U5 -r1.2 embeddable-document.c --- src/bonobo/embeddable-document.c 30 Oct 2003 05:52:06 -0000 1.2 +++ src/bonobo/embeddable-document.c 30 Nov 2003 03:34:39 -0000 @@ -132,11 +132,11 @@ sp_bonobo_stream_read (Bonobo_Stream str
do { Bonobo_Stream_read (stream, STREAM_CHUNK_SIZE, &iobuf, &ev);
if (ev._major != CORBA_NO_EXCEPTION) { - if (* buffer != NULL) g_free (* buffer); + g_free (* buffer); len = -1; break; }
* buffer = g_realloc (* buffer, len + iobuf->_length); @@ -204,11 +204,11 @@ sp_embeddable_document_ps_save (BonoboPe
document = SP_EMBEDDABLE_DOCUMENT (closure);
/* FIXME: super dirty but functional */ do { - if (filename) g_free (filename); + g_free (filename); filename = g_strdup ("inkscapeXXXXXX"); fd = mkstemp (filename);
} while (fd == -1);
Index: src/dialogs/stroke-style.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/dialogs/stroke-style.c,v retrieving revision 1.20 diff -d -p -U5 -r1.20 stroke-style.c --- src/dialogs/stroke-style.c 29 Nov 2003 22:03:24 -0000 1.20 +++ src/dialogs/stroke-style.c 30 Nov 2003 03:34:39 -0000 @@ -1101,11 +1101,11 @@ sp_stroke_style_scale_line (SPWidget *sp g_snprintf (c, 32, "%g", dist); sp_repr_css_set_property (css, "stroke-width", c); /* Set dash */ sp_stroke_style_set_scaled_dash (css, ndash, dash, offset, dist); sp_repr_css_change_recursive (SP_OBJECT_REPR (i->data), css, "style"); - if (dash) g_free (dash); + g_free (dash); } } else { for (r = reprs; r != NULL; r = r->next) { double length; double *dash, offset; @@ -1115,11 +1115,11 @@ sp_stroke_style_scale_line (SPWidget *sp sp_convert_distance (&length, sp_unit_selector_get_unit (us), SP_PS_UNIT); g_snprintf (c, 32, "%g", length * 1.25); sp_repr_css_set_property (css, "stroke-width", c); sp_stroke_style_set_scaled_dash (css, ndash, dash, offset, length); sp_repr_css_change_recursive ((SPRepr *) r->data, css, "style"); - if (dash) g_free (dash); + g_free (dash); } }
sp_repr_css_attr_unref (css); if (spw->inkscape) sp_document_done (SP_WIDGET_DOCUMENT (spw)); Index: src/dialogs/text-edit.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/dialogs/text-edit.c,v retrieving revision 1.5 diff -d -p -U5 -r1.5 text-edit.c --- src/dialogs/text-edit.c 27 Nov 2003 00:55:38 -0000 1.5 +++ src/dialogs/text-edit.c 30 Nov 2003 03:34:39 -0000 @@ -637,11 +637,11 @@ sp_text_edit_dialog_text_changed (GtkTex if (str && *str) { sp_font_preview_set_phrase (SP_FONT_PREVIEW (preview), str); } else { sp_font_preview_set_phrase (SP_FONT_PREVIEW (preview), NULL); } - if (str) g_free (str); + g_free (str);
if (text) { gtk_widget_set_sensitive (apply, TRUE); } gtk_widget_set_sensitive (def, TRUE); Index: src/helper/sodipodi-ctrl.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/helper/sodipodi-ctrl.c,v retrieving revision 1.3 diff -d -p -U5 -r1.3 sodipodi-ctrl.c --- src/helper/sodipodi-ctrl.c 30 Oct 2003 11:33:16 -0000 1.3 +++ src/helper/sodipodi-ctrl.c 30 Nov 2003 03:34:39 -0000 @@ -298,11 +298,11 @@ sp_ctrl_build_cache (SPCtrl *ctrl) } else { sr = fr; sg = fg; sb = fb; sa = fa; }
side = (ctrl->span * 2 +1); c = ctrl->span ; - if (ctrl->cache) g_free (ctrl->cache); + g_free (ctrl->cache); size = (side) * (side) * 4; ctrl->cache = (guchar*)g_malloc (size); if (side < 2) return; switch (ctrl->shape) { Index: src/modules/ps.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/modules/ps.c,v retrieving revision 1.8 diff -d -p -U5 -r1.8 ps.c --- src/modules/ps.c 16 Nov 2003 23:37:19 -0000 1.8 +++ src/modules/ps.c 30 Nov 2003 03:34:39 -0000 @@ -854,12 +854,12 @@ sp_ps_print_image (FILE *ofp, guchar *px #if 0 fprintf (ofp, "showpage\n"); g_free (data); #endif
- if (packb != NULL) g_free (packb); - if (plane != NULL) g_free (plane); + g_free (packb); + g_free (plane);
#if 0 if (ferror (ofp)) { g_message (_("write error occured")); Index: src/widgets/font-selector.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/widgets/font-selector.c,v retrieving revision 1.3 diff -d -p -U5 -r1.3 font-selector.c --- src/widgets/font-selector.c 16 Nov 2003 23:37:19 -0000 1.3 +++ src/widgets/font-selector.c 30 Nov 2003 03:34:39 -0000 @@ -617,11 +617,11 @@ sp_font_preview_set_rgba32 (SPFontPrevie }
void sp_font_preview_set_phrase (SPFontPreview *fprev, const gchar *phrase) { - if (fprev->phrase) g_free (fprev->phrase); + g_free (fprev->phrase); if (phrase) { fprev->phrase = g_strdup (phrase); } else { fprev->phrase = NULL; } Index: src/widgets/gradient-image.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/widgets/gradient-image.c,v retrieving revision 1.2 diff -d -p -U5 -r1.2 gradient-image.c --- src/widgets/gradient-image.c 30 Oct 2003 20:48:05 -0000 1.2 +++ src/widgets/gradient-image.c 30 Nov 2003 03:34:39 -0000 @@ -149,11 +149,11 @@ sp_gradient_image_size_allocate (GtkWidg image = SP_GRADIENT_IMAGE (widget);
widget->allocation = *allocation;
if (GTK_WIDGET_REALIZED (widget)) { - if (image->px) g_free (image->px); + g_free (image->px); image->px = g_new (guchar, 3 * VBLOCK * allocation->width); }
sp_gradient_image_update (image); } Index: src/xml/repr-action.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/xml/repr-action.c,v retrieving revision 1.5 diff -d -p -U5 -r1.5 repr-action.c --- src/xml/repr-action.c 29 Nov 2003 02:55:37 -0000 1.5 +++ src/xml/repr-action.c 30 Nov 2003 03:34:39 -0000 @@ -228,12 +228,11 @@ coalesce_chgattr (SPReprAction *action) { /* ensure changes are continuous */ if (strcmp(action->act.chgattr.oldval, iter->act.chgattr.newval)) break;
- if (action->act.chgattr.oldval) - g_free (action->act.chgattr.oldval); + g_free (action->act.chgattr.oldval);
action->act.chgattr.oldval = iter->act.chgattr.oldval; iter->act.chgattr.oldval = NULL; free_action (iter); prev->next = next; @@ -262,12 +261,11 @@ coalesce_chgcontent (SPReprAction *actio if ( action->repr == iter->repr ) { /* ensure changes are continuous */ if (strcmp(action->act.chgcontent.oldval, iter->act.chgcontent.newval)) break;
- if (action->act.chgcontent.oldval) - g_free (action->act.chgcontent.oldval); + g_free (action->act.chgcontent.oldval);
action->act.chgcontent.oldval = iter->act.chgcontent.oldval;
iter->act.chgcontent.oldval = NULL; @@ -429,20 +427,16 @@ free_action (SPReprAction *action) sp_repr_unref (action->act.del.child); if (action->act.del.ref) sp_repr_unref (action->act.del.ref); break; case SP_REPR_ACTION_CHGATTR: - if (action->act.chgattr.oldval) - g_free (action->act.chgattr.oldval); - if (action->act.chgattr.newval) - g_free (action->act.chgattr.newval); + g_free (action->act.chgattr.oldval); + g_free (action->act.chgattr.newval); break; case SP_REPR_ACTION_CHGCONTENT: - if (action->act.chgcontent.oldval) - g_free (action->act.chgcontent.oldval); - if (action->act.chgcontent.newval) - g_free (action->act.chgcontent.newval); + g_free (action->act.chgcontent.oldval); + g_free (action->act.chgcontent.newval); break; case SP_REPR_ACTION_CHGORDER: sp_repr_unref (action->act.chgorder.child); if (action->act.chgorder.oldref) sp_repr_unref (action->act.chgorder.oldref); Index: src/xml/repr.c =================================================================== RCS file: /cvsroot/inkscape/inkscape/src/xml/repr.c,v retrieving revision 1.5 diff -d -p -U5 -r1.5 repr.c --- src/xml/repr.c 16 Nov 2003 23:37:20 -0000 1.5 +++ src/xml/repr.c 30 Nov 2003 03:34:39 -0000 @@ -173,11 +173,11 @@ repr_finalize (SPRepr *repr) for (rl = repr->listeners; rl; rl = rl->next) { if (rl->vector->destroy) (* rl->vector->destroy) (repr, rl->data); } while (repr->children) sp_repr_remove_child (repr, repr->children); while (repr->attributes) sp_repr_remove_attribute (repr, repr->attributes); - if (repr->content) g_free (repr->content); + g_free (repr->content); while (repr->listeners) sp_repr_remove_listener (repr, repr->listeners); }
static void repr_doc_finalize (SPRepr *repr) @@ -859,14 +859,14 @@ sp_repr_merge (SPRepr *repr, const SPRep g_return_val_if_fail (repr != NULL, FALSE); g_return_val_if_fail (src != NULL, FALSE); g_return_val_if_fail (key != NULL, FALSE);
if (src->content) { - if (repr->content) g_free (repr->content); + g_free (repr->content); repr->content = g_strdup (src->content); } else { - if (repr->content) g_free (repr->content); + g_free (repr->content); repr->content = NULL; } for (child = src->children; child != NULL; child = child->next) { SPRepr *rch;
On Sun, 30 Nov 2003, Peter Moulder wrote:
This is a low-priority cleanup that may be objectionable, so I'll post here first.
According to the g_free documentation, g_free accepts a NULL argument (as a no-op), yet much code explicitly checks for NULL before calling g_free.
Disadvantage of the transformation:
Involves a function call even in the NULL case (unless g_free is a macro or inline function that tests for NULL before passing to the "real" version). I've left a couple of places untransformed where it looks like NULL has a significant probability. If NULL is unlikely, then we're better off having more readable code [and saving a couple of bytes of object code].
May make diff3 against sodipodi code harder.
Strong candidates for applying the transformation are where the tested pointer is something long like `style->text->font_family.value', and src/module.c where it uses a IF_NOT_NULL_FREE macro.
Btw, some (but not all) of the searching for this pattern was using a regexp to reduce the probability of accidentally transforming `if (foo) g_free (bar)' for distinct `foo' and `bar'.
Any comments on where this transformation (removing the `if') should be applied? In absence of comments, I'll probably commit the patch in its current form (other than ChangeLog date).
pjrm.
Thanks for identifying the concerns about this and posting it as a patch before applying it. It looks like you've researched the change well, and I don't have any issues with applying it. As a fringe benefit, it looks like this also eliminates a good chunk of the bracket-less if's that makes Jon Cruz squeal. ;-)
Bryce
Peter Moulder wrote:
Disadvantage of the transformation:
- Involves a function call even in the NULL case (unless g_free is a macro or inline function that tests for NULL before passing to the "real" version).
My question here is: "So, what's the problem with that?"
What's the extra overhead of the function call? milliseconds? microseconds? How often does a function call with null happen? once a minute? 1,000 times a second?
Otherwise... that sounds like a premature optimization kind of concern. (BTW, I think it originated to keep older C memory libraries happy)
participants (3)
-
Bryce Harrington
-
Jon A. Cruz
-
Peter Moulder