Created attachment 350899 [details] [review]
toolbar: Don't leak previous allocations

When undo_active/redo_active is FALSE, undo_label/redo_label
was getting overwritten irrespective of whether they were
NULL or not.

Let those values be updated only when they are NULL and thus
avoid possible leaks.

Review of attachment 350899 [details] [review]:

Awesome, thanks!

::: src/nautilus-toolbar.c
@@ +855,3 @@
      */
+
+    if (undo_label == NULL)

Braces and four-space indents.

Created attachment 350902 [details] [review]
toolbar: Don't leak previous allocations

When undo_active/redo_active is FALSE, undo_label/redo_label
was getting overwritten irrespective of whether they were
NULL or not.

Let those values be updated only when they are NULL and thus
avoid possible leaks.

Sorry, my editor defaults to GNU style.

Review of attachment 350902 [details] [review]:

Discussed to oblivion on IRC, LGTM.

Comment on attachment 350902 [details] [review]
toolbar: Don't leak previous allocations

Ah, sorry, this breaks the logic of the code.

Review of attachment 350902 [details] [review]:

::: src/nautilus-toolbar.c
@@ -854,3 @@
     /* Set the label of the undo and redo menu items, and activate them appropriately
      */
-    undo_label = undo_active && undo_label != NULL ? undo_label : g_strdup (_("_Undo"));

What needs to happen here is for undo_label to be set to “Undo” if !undo_active. So you need to figure out the deallocation in that case.

@@ +857,3 @@
+    {
+        undo_label = g_strdup (_("_Undo"));
+    }

If you test the code, you’ll see how it breaks.

Created attachment 350984 [details]
pre-patch screencast

Created attachment 350985 [details]
post-patch screencast

Created attachment 350989 [details] [review]
toolbar: Don't leak previous allocations

When undo_active/redo_active is FALSE, undo_label/redo_label
was getting overwritten irrespective of whether they were
NULL or not.

Let the label values be freed incase they are updated.

Review of attachment 350989 [details] [review]:

Okay, for real this time. :)

Attachment 350989 [details] pushed as ec52282 - toolbar: Don't leak previous allocations