libnm: add option for direct STRV properties to preseve/distinguish empty arrays

For most strv or string properties, we cannot distinguish between
NULL/unset/default and empty.

It would be difficult to enter in nmcli or grasp how it differs. There
are probably many bugs, where we accept empty strings, and fail to
handle them correctly.

Anyway. For most strv arrays, and empty array and NULL/unset/default are
treated the same. That means, g_object_get() tends to always return NULL
(never an empty strv array) and g_object_set() of an empty strv array
will internally leave the GArray at NULL.

For a few properties, there is a difference. See "ipv[46].dns-options".
See also "clear_emptyunset_fcn" hook in libnm-setting.

Add a way to handle such strv properties with the "direct" mechanism.
This commit is contained in:
Thomas Haller 2023-11-16 12:18:47 +01:00
parent 189bddc99b
commit 9f4cd6b03f
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
3 changed files with 45 additions and 22 deletions

View file

@ -729,6 +729,24 @@ out_take:
return nm_strdup_reset_take(dst, s);
}
static gboolean
_property_direct_set_strv(const NMSettInfoSetting *sett_info,
const NMSettInfoProperty *property_info,
NMSetting *setting,
const char *const *strv)
{
NMValueStrv *p_val = _nm_setting_get_private_field(setting, sett_info, property_info);
if (!property_info->direct_strv_preserve_empty && strv && !strv[0])
strv = NULL;
if (nm_strvarray_equal_strv(p_val->arr, strv, -1))
return FALSE;
nm_strvarray_set_strv_full(&p_val->arr, strv, property_info->direct_strv_preserve_empty);
return TRUE;
}
void
_nm_setting_property_get_property_direct(GObject *object,
guint prop_id,
@ -817,7 +835,12 @@ _nm_setting_property_get_property_direct(GObject *object,
{
const NMValueStrv *p_val = _nm_setting_get_private_field(setting, sett_info, property_info);
g_value_take_boxed(value, nm_strvarray_get_strv_notempty_dup(p_val->arr, NULL));
g_value_take_boxed(
value,
nm_strvarray_get_strv_full_dup(p_val->arr,
NULL,
FALSE,
property_info->direct_strv_preserve_empty));
return;
}
default:
@ -956,17 +979,9 @@ _nm_setting_property_set_property_direct(GObject *object,
goto out_notify;
}
case NM_VALUE_TYPE_STRV:
{
NMValueStrv *p_val = _nm_setting_get_private_field(setting, sett_info, property_info);
const char *const *v;
v = g_value_get_boxed(value);
if (nm_strvarray_equal_strv(p_val->arr, v, -1))
if (!_property_direct_set_strv(sett_info, property_info, setting, g_value_get_boxed(value)))
return;
nm_strvarray_set_strv(&p_val->arr, v);
goto out_notify;
}
default:
goto out_fail;
}
@ -1281,7 +1296,7 @@ _nm_setting_property_to_dbus_fcn_direct(_NM_SETT_INFO_PROP_TO_DBUS_FCN_ARGS _nm_
(const NMValueStrv *) _nm_setting_get_private_field(setting, sett_info, property_info);
if (!val->arr)
return NULL;
if (val->arr->len == 0) {
if (!property_info->direct_strv_preserve_empty && val->arr->len == 0) {
/* This property does not treat empty strv arrays special. No need
* to export the value on D-Bus. */
return NULL;
@ -1650,7 +1665,6 @@ _nm_setting_property_from_dbus_fcn_direct(_NM_SETT_INFO_PROP_FROM_DBUS_FCN_ARGS
}
case NM_VALUE_TYPE_STRV:
{
NMValueStrv *p_val;
gs_free const char **ss = NULL;
gsize ss_len;
@ -1661,13 +1675,10 @@ _nm_setting_property_from_dbus_fcn_direct(_NM_SETT_INFO_PROP_FROM_DBUS_FCN_ARGS
ss = g_variant_get_strv(value, &ss_len);
nm_assert(ss_len <= G_MAXUINT);
nm_assert(NM_PTRARRAY_LEN(ss) == ss_len);
p_val = _nm_setting_get_private_field(setting, sett_info, property_info);
if (nm_strvarray_equal_strv(p_val->arr, ss, ss_len))
if (!_property_direct_set_strv(sett_info, property_info, setting, ss))
goto out_unchanged;
nm_strvarray_set_strv(&p_val->arr, ss);
goto out_notify;
}
default:
@ -2590,11 +2601,13 @@ _nm_setting_property_compare_fcn_direct(_NM_SETT_INFO_PROP_COMPARE_FCN_ARGS _nm_
const GArray *a = v_a->arr;
const GArray *b = v_b->arr;
/* NULL and empty are treated identical. Coerce to NULL. */
if (a && a->len == 0)
a = NULL;
if (b && b->len == 0)
b = NULL;
if (!property_info->direct_strv_preserve_empty) {
/* NULL and empty are treated identical. Coerce to NULL. */
if (a && a->len == 0)
a = NULL;
if (b && b->len == 0)
b = NULL;
}
return nm_strvarray_equal(a, b);
}
default:

View file

@ -4736,6 +4736,8 @@ test_setting_metadata(void)
g_assert(sip->param_spec);
g_assert(!NM_FLAGS_HAS(sip->param_spec->flags, NM_SETTING_PARAM_SECRET));
}
if (sip->direct_strv_preserve_empty)
g_assert(sip->property_type->direct_type == NM_VALUE_TYPE_STRV);
if (sip->direct_set_string_mac_address_len != 0) {
g_assert(NM_IN_SET(sip->property_type,

View file

@ -818,6 +818,14 @@ struct _NMSettInfoProperty {
/* Whether the string property is implemented as a (downcast) NMRefString. */
bool direct_string_is_refstr : 1;
/* Usually, for strv arrays (NM_VALUE_TYPE_STRV, NMValueStrv) there is little
* difference between NULL/unset and empty arrays. E.g. g_object_get() will
* return NULL and never an empty strv array.
*
* By setting this flag, this property treats a NULL array different from
* an empty array. */
bool direct_strv_preserve_empty : 1;
/* Usually, properties that are set to the default value for the GParamSpec
* are not serialized to GVariant (and NULL is returned by to_dbus_data().
* Set this flag to force always converting the property even if the value