gdbus tool "parse me harder" is a bit bogus
Submitted by Allison (desrt)
Assigned to David Zeuthen
Link to original bug (#715031)
Description
The "parse me harder" functionality of the GDBus tool is vaguely bogus:
static GVariant * _g_variant_parse_me_harder (GVariantType *type, const gchar *given_str, GError **error) { GVariant *value; gchar *s; guint n; GString *str;
str = g_string_new ("""); for (n = 0; given_str[n] != '\0'; n++) { if (G_UNLIKELY (given_str[n] == '"')) g_string_append (str, "\""); else g_string_append_c (str, given_str[n]); } g_string_append_c (str, '"'); s = g_string_free (str, FALSE);
value = g_variant_parse (type, s, NULL, NULL, error); g_free (s);
return value; }
First of all, it could be much more easily rewritten thus:
parse_me_harder (type, str) { if (type == string) return g_variant_new_string (str); else throw "this will never work"; }
Second of all, it makes it absolutely impossible to have a GVariant parse error when:
-
the method you're sending to is expecting a string
-
you're sending a signal, in any case
and yet there is error handling code in the tool for these cases (which is effectively dead).
The approach taken by the GSettings tool is a bit more sane here:
new = g_variant_parse (type, global_value, NULL, NULL, &error);
/* If that didn't work and the type is string then we should assume
- that the user is just trying to set a string directly and forgot
- the quotes (or had them consumed by the shell).
- If the user started with a quote then we assume that some deeper
- problem is at play and we want the failure in that case.
- Consider:
- gsettings set x.y.z key "'i don't expect this to work'"
- Note that we should not just add quotes and try parsing again, but
- rather assume that the user is providing us with a bare string.
- Assume we added single quotes, then consider this case:
- gsettings set x.y.z key "i'd expect this to work"
- A similar example could be given for double quotes.
- Avoid that whole mess by just using g_variant_new_string(). */ if (new == NULL && g_variant_type_equal (type, G_VARIANT_TYPE_STRING) && global_value[0] != ''' && global_value[0] != '"') { g_clear_error (&error); new = g_variant_new_string (global_value); }
in the signal case where we have no type information, I propose two possibilities:
-
see if we can find a D-Bus interface file in the usual location that describes the signal that we are attempting to emit and check if it has type information
-
never try to parse me harder on signals -- method calls are really the more common case here so it's where we get the most benefit