info struct bug Fixes, add tagged update fields

* tag was always included even when it didn't need to be. Now only
  include it when tag specified, flagged, or includes an icon.
  This messed up the existing update fields system.
* The update fields system has been changed to allow updating by tag
  instead of the translated label. By label still works, however.
  I think it would be best to switch to using tags in the future.
* info_flatten() calls flatten_shell_param() after flatten_group()
  which could cause a field name or tag to be used after it was freed.
  Created special free functions to handle this after all is used.

Signed-off-by: Burt P <pburt0@gmail.com>

3 files changed, 75 insertions, 28 deletions

diff --git a/hardinfo/info.c b/hardinfo/info.c
index c3ace958..fd2eb959 100644
--- a/hardinfo/info.c
+++ b/hardinfo/info.c
@@ -198,6 +198,29 @@ static const GCompareFunc sort_functions[INFO_GROUP_SORT_MAX] = {
     [INFO_GROUP_SORT_TAG_DESCENDING] = info_field_cmp_tag_descending,
 };
 
+static void _field_free_strings(struct InfoField *field)
+{
+    if (field) {
+        if (field->free_value_on_flatten)
+            g_free((gchar *)field->value);
+        if (field->free_name_on_flatten)
+            g_free((gchar *)field->name);
+        g_free(field->tag);
+    }
+}
+
+static void _group_free_field_strings(struct InfoGroup *group)
+{
+    guint i;
+    if (group && group->fields) {
+        for (i = 0; i < group->fields->len; i++) {
+            struct InfoField field;
+            field = g_array_index(group->fields, struct InfoField, i);
+            _field_free_strings(&field);
+        }
+    }
+}
+
 static void flatten_group(GString *output, const struct InfoGroup *group, guint group_count)
 {
     guint i;
@@ -212,27 +235,23 @@ static void flatten_group(GString *output, const struct InfoGroup *group, guint
     if (group->fields) {
         for (i = 0; i < group->fields->len; i++) {
             struct InfoField *field = &g_array_index(group->fields, struct InfoField, i);
-            gchar tag[256] = "";
+            gchar tmp_tag[256] = ""; /* for generated tag */
 
-            if (field->tag)
-                strncpy(tag, field->tag, 255);
-            else
-                snprintf(tag, 255, "ITEM%d-%d", group_count, i);
+            const gchar *tp = field->tag;
+            gboolean tagged = !!tp;
+            gboolean flagged = field->highlight || field->report_details;
+            if (!tp) {
+                snprintf(tmp_tag, 255, "ITEM%d-%d", group_count, i);
+                tp = tmp_tag;
+            }
 
-            if (*tag != 0 || field->highlight || field->report_details)
+            if (tagged || flagged || field->icon)
                 g_string_append_printf(output, "$%s%s%s$",
                     field->highlight ? "*" : "",
                     field->report_details ? "!" : "",
-                    tag);
+                    tp);
 
             g_string_append_printf(output, "%s=%s\n", field->name, field->value);
-
-            if (field->free_value_on_flatten)
-                g_free((gchar *)field->value);
-            if (field->free_name_on_flatten)
-                g_free((gchar *)field->name);
-
-            g_free(field->tag);
         }
     } else if (group->computed) {
         g_string_append_printf(output, "%s\n", group->computed);
@@ -248,16 +267,25 @@ static void flatten_shell_param(GString *output, const struct InfoGroup *group,
 
     for (i = 0; i < group->fields->len; i++) {
         struct InfoField *field = &g_array_index(group->fields, struct InfoField, i);
+        gchar tmp_tag[256] = ""; /* for generated tag */
 
+        const gchar *tp = field->tag;
+        gboolean tagged = !!tp;
+        if (!tp) {
+            snprintf(tmp_tag, 255, "ITEM%d-%d", group_count, i);
+            tp = tmp_tag;
+        }
 
         if (field->update_interval) {
-            g_string_append_printf(output, "UpdateInterval$%s=%d\n",
-                field->name, field->update_interval);
+            g_string_append_printf(output, "UpdateInterval$%s%s%s=%d\n",
+                tagged ? tp : "", tagged ? "$" : "", /* tag and close or nothing */
+                field->name,
+                field->update_interval);
         }
 
         if (field->icon) {
-            g_string_append_printf(output, "Icon$ITEM%d-%d$=%s\n",
-                group_count, i, field->icon);
+            g_string_append_printf(output, "Icon$%s$=%s\n",
+                tp, field->icon);
         }
     }
 }
@@ -310,6 +338,8 @@ gchar *info_flatten(struct Info *info)
             flatten_group(values, group, i);
             flatten_shell_param(shell_param, group, i);
 
+            _group_free_field_strings(group);
+
             if (group->fields)
                 g_array_free(group->fields, TRUE);
         }
diff --git a/modules/computer.c b/modules/computer.c
index cc3e9be3..99257e9f 100644
--- a/modules/computer.c
+++ b/modules/computer.c
@@ -110,31 +110,42 @@ gchar *hi_more_info(gchar * entry)
     return g_strdup_printf("[%s]", entry);
 }
 
+/* a g_str_equal() where either may be null */
+#define g_str_equal0(a,b) (g_strcmp0(a,b) == 0)
+
 gchar *hi_get_field(gchar * field)
 {
+    gchar *tag, *label;
+    key_get_components(field, NULL, &tag, NULL, &label, NULL, TRUE);
+
     gchar *tmp;
 
-    if (g_str_equal(field, _("Memory"))) {
+    if (g_str_equal0(label, _("Memory"))) {
         MemoryInfo *mi = computer_get_memory();
         tmp = g_strdup_printf(_("%dMB (%dMB used)"), mi->total, mi->used);
         g_free(mi);
-    } else if (g_str_equal(field, _("Uptime"))) {
+    } else if (g_str_equal0(label, _("Uptime"))) {
         tmp = computer_get_formatted_uptime();
-    } else if (g_str_equal(field, _("Date/Time"))) {
+    } else if (g_str_equal0(label, _("Date/Time"))) {
         time_t t = time(NULL);
 
         tmp = g_new0(gchar, 64);
         strftime(tmp, 64, "%c", localtime(&t));
-    } else if (g_str_equal(field, _("Load Average"))) {
+    } else if (g_str_equal0(label, _("Load Average"))) {
         tmp = computer_get_formatted_loadavg();
-    } else if (g_str_equal(field, _("Available entropy in /dev/random"))) {
+    } else if (g_str_equal0(tag, "entropy")) {
         tmp = computer_get_entropy_avail();
     } else {
-        gchar *info = moreinfo_lookup_with_prefix("DEV", field);
+        gchar *info = NULL;
+        if (tag)
+            info = moreinfo_lookup_with_prefix("DEV", tag);
+        else if (label)
+            info = moreinfo_lookup_with_prefix("DEV", label);
+
         if (info)
             tmp = g_strdup(info);
         else
-            tmp = g_strdup_printf("Unknown field: %s", field);
+            tmp = g_strdup_printf("Unknown field: [tag: %s] label: %s", tag ? tag : "(none)", label ? label : "(empty)");
     }
     return tmp;
 }
@@ -600,7 +611,7 @@ gchar *callback_security(void)
 
     info_add_group(
         info, _("Health"),
-        info_field_update(_("Available entropy in /dev/random"), 1000),
+        info_field_update(_("Available entropy in /dev/random"), 1000, .tag = g_strdup("entropy") ),
         info_field_last());
 
     info_add_group(
diff --git a/shell/shell.c b/shell/shell.c
index a7bf51c4..674280f7 100644
--- a/shell/shell.c
+++ b/shell/shell.c
@@ -1126,7 +1126,13 @@ static void group_handle_special(GKeyFile *key_file,
 
             ms = g_key_file_get_integer(key_file, group, key, NULL);
 
-            fu->field_name = g_strdup(g_utf8_strchr(key, -1, '$') + 1);
+            /* Old style used just the label which has to be checked by translating it,
+             * and potentially could by ambiguous.
+             * New style can use tag or label. If new style including a tag,
+             * send both tag and label and let the hi_get_field() function use
+             * key_get_components() to split it. */
+            const gchar *chk = g_utf8_strchr(key, -1, '$');
+            fu->field_name = g_strdup(key_is_flagged(chk) ? chk : chk + 1);
             fu->entry = entry;
 
             sfutbl = g_new0(ShellFieldUpdateSource, 1);
@@ -1537,7 +1543,7 @@ static void module_selected_show_info_detail(GKeyFile *key_file,
 
                 if (entry && entry->fieldfunc && value && g_str_equal(value, "...")) {
                     g_free(value);
-                    value = entry->fieldfunc(name);
+                    value = entry->fieldfunc(keys[j]);
                 }
 
                 key_markup =