cli: handle string properties that can both be empty and %NULL

The default value of a string property (almost?) always should be
%NULL, which means the value is absent and not specified.
That is necessary because adding new properties must be backward
compatible. That means, after upgrade those properties will have their
value unset. In these cases, %NULL really translates to some property
dependant behavior (like not using the value, or using a special default
value).

For example leaving "ethernet.cloned-mac-address" unset really means
"preserve", with the twist that %NULL can be overridden by a global
connection default.

For most string properties, a value can only be unset (%NULL) or set to
a non-empty string. nm_connection_verify() enforces that.

However, for some properties, it makes sense to allow both unset and the
empty word "" as value. This is the case if a property can have it's
value overridden by a global connection default, or if we need the
differentiation between having a value unset and having it set to the empty
word.

We would usually avoid allowing the empty word beside %NULL, because
that makes it hard to express the difference on the command line of
nmcli or in a UI text entry field. In the "ethernet.cloned-mac-address"
example, "" is not necessary nor sensible.

However, for some properties really all string values may be possible (including
"") and also unset/%NULL. Then, we need some form of escaping/mangling,
to allow to express all possible values. The chosen style here is that
on nmcli input field "" means %NULL, while a word with all white space
stands for the word with one less white space characters.

This is still unused, but I think it makes sense for some properties.
I initially added this for "connection.mud-url", but a valid MUD-URL
always must start with "https://", so not all strings are possible
to begin with. So to explicitly express that no MUD-URL should be set,
we will instead introduce a special word "none", and not use the empty
word, due to the oddities discussed here. However, I think this may
well make sense for some properties where all strings are valid.
This commit is contained in:
Thomas Haller 2020-04-25 08:22:32 +02:00
parent e302f5ff77
commit e9ee4e39f1
2 changed files with 43 additions and 8 deletions

View file

@ -783,6 +783,17 @@ _coerce_str_emptyunset (NMMetaAccessorGetType get_type,
return cstr; return cstr;
} }
#define RETURN_STR_EMPTYUNSET(get_type, is_default, cstr) \
G_STMT_START { \
char *_str = NULL; \
const char *_cstr; \
\
_cstr = _coerce_str_emptyunset ((get_type), (is_default), (cstr), &_str); \
if (_str) \
RETURN_STR_TO_FREE (_str); \
RETURN_STR_TEMPORARY (_cstr); \
} G_STMT_END
static gboolean static gboolean
_is_default (const NMMetaPropertyInfo *property_info, _is_default (const NMMetaPropertyInfo *property_info,
NMSetting *setting) NMSetting *setting)
@ -836,6 +847,19 @@ _get_fcn_gobject_impl (const NMMetaPropertyInfo *property_info,
|| ( gtype_prop == G_TYPE_STRV || ( gtype_prop == G_TYPE_STRV
&& !glib_handles_str_transform)); && !glib_handles_str_transform));
if (gtype_prop == G_TYPE_STRING) {
nm_assert (glib_handles_str_transform);
nm_assert (!handle_emptyunset);
if ( property_info->property_typ_data
&& property_info->property_typ_data->subtype.gobject_string.handle_emptyunset) {
/* This string property can both be empty and NULL. We need to
* signal them differently. */
cstr = g_value_get_string (&val);
nm_assert ((!!is_default) == (cstr == NULL));
RETURN_STR_EMPTYUNSET (get_type, is_default, NULL);
}
}
if (glib_handles_str_transform) if (glib_handles_str_transform)
RETURN_STR_TEMPORARY (g_value_get_string (&val)); RETURN_STR_TEMPORARY (g_value_get_string (&val));
@ -857,15 +881,9 @@ _get_fcn_gobject_impl (const NMMetaPropertyInfo *property_info,
if (strv && strv[0]) if (strv && strv[0])
RETURN_STR_TO_FREE (g_strjoinv (",", (char **) strv)); RETURN_STR_TO_FREE (g_strjoinv (",", (char **) strv));
/* special hack for handling properties that can be empty and unset
* (see multilist.clear_emptyunset_fcn). */
if (handle_emptyunset) { if (handle_emptyunset) {
char *str = NULL; /* we need to express empty lists from unset lists differently. */
RETURN_STR_EMPTYUNSET (get_type, is_default, NULL);
cstr = _coerce_str_emptyunset (get_type, is_default, NULL, &str);
if (str)
RETURN_STR_TO_FREE (str);
RETURN_STR_TEMPORARY (cstr);
} }
return ""; return "";
@ -1183,6 +1201,22 @@ _set_fcn_gobject_string (ARGS_SET_FCN)
return _gobject_property_reset_default (setting, property_info->property_name); return _gobject_property_reset_default (setting, property_info->property_name);
if (property_info->property_typ_data) { if (property_info->property_typ_data) {
if (property_info->property_typ_data->subtype.gobject_string.handle_emptyunset) {
if ( value
&& value[0]
&& NM_STRCHAR_ALL (value, ch, ch == ' ')) {
/* this string property can both be %NULL and empty. To express that, we coerce
* a value of all whitespaces to dropping the first whitespace. That means,
* " " gives "", " " gives " ", and so on.
*
* This way the user can set the string value to "" (meaning NULL) and to
* " " (meaning ""), and any other string.
*
* This is and non-obvious escaping mechanism. But out of all the possible
* solutions, it seems the most sensible one. */
value++;
}
}
if (property_info->property_typ_data->subtype.gobject_string.validate_fcn) { if (property_info->property_typ_data->subtype.gobject_string.validate_fcn) {
value = property_info->property_typ_data->subtype.gobject_string.validate_fcn (value, &to_free, error); value = property_info->property_typ_data->subtype.gobject_string.validate_fcn (value, &to_free, error);
if (!value) if (!value)

View file

@ -257,6 +257,7 @@ struct _NMMetaPropertyTypData {
} gobject_int; } gobject_int;
struct { struct {
const char *(*validate_fcn) (const char *value, char **out_to_free, GError **error); const char *(*validate_fcn) (const char *value, char **out_to_free, GError **error);
bool handle_emptyunset:1;
} gobject_string; } gobject_string;
struct { struct {
bool legacy_format:1; bool legacy_format:1;