There is a quest to move away from the GObject/GValue based setters.
Add _nm_setting_property_from_dbus_fcn_direct(), which can parse
the GVariant and use the direct_type to set the property.
Note that for backward compatibility, we still need
_nm_property_variant_to_gvalue() to convert alternative GVariant
types to the destination value. This means, as before, on the D-Bus
API a property of a certain type can be represented as various D-Bus
types.
This is a normalization employed by NMSettingIPConfig.gateway.
Also rework NMSettingIPConfig.set_property() to no longer assert against
valid input. We want to pass there untrusted strings from D-Bus,
asserting is a horrible idea. Instead, either normalize the string or
keep the invalid text that will be rejected by verify().
A MAC address is a relatively common "type". The GObject property is of type string,
but the D-Bus type is a bytestring ("ay"). We will need a special NMSettInfoPropertType.
Note that like most implementations, the from-dbus implementation still is based
on GObject setters. This will change in the future.
Also note that the previous compare function was
_nm_setting_property_compare_fcn_default(). That is, it used to convert
the property to GVariant and compare those. The conversion to GVariant
in that case normalizes the string (e.g. it is case insensitive). Also,
only properties could be compared which were also convertible to D-Bus
(which is probably fine, because there is no guarantee the profiles that
don't verify can be compared).
The code now uses the direct comparison of the strings. That mostly
preserves the case-insensitivity of the previous comparison, because
the property setters for mac addresses all use
_nm_utils_hwaddr_canonical_or_invalid() to normalize the strings.
This is subtle, but still correct. Note that this will improve later,
by ensuring that the property setters for mac addresses automatically
perform the right normalization.
The aim is that properties have a "type", that is, that similar
properties share a common behavior and appearance.
Most properties of type "mac-address" normalize the string in the
GObject property setter. Three don't. Let them also do that.
This is also relevant, because the compare function for mac-addresses
(_nm_setting_property_compare_fcn_default()) converts the properties
first to a "ay" GVariant. Which means the comparison is case
insensitive. Normalizing the values in the setter avoids that
inconsistency.
On architectures where "char" is signed, the check "ch < ' '" is
also TRUE for non-ASCII characters greater than 127. This is an
easy mistake to make. Fix it by using nm_ascii_is_control() which
gets this right.
It's a bug, but possibly not too bad because unnecesarily escaping
a UTF-8 characters is not a severe problem, because the user anyway must
be prepared to unescape the string.
When looking at a property, it should always be clear how it is handled.
Also the "default" action should be an explicit hook.
Add _nm_setting_property_from_dbus_fcn_gprop() and set that as
from_dbus_fcn() callback to handle the "default" case which us
build around g_object_set_property().
While this adds lines of code, I think it makes the code easier to
understand. Basically, to convert a GVariant to a property, now all
properties call their from_dbus_fcn() handler, there is no special casing.
And the gprop-hook is only called for properties that are using
_nm_setting_property_from_dbus_fcn_gprop(). So, you can reason about
these two functions at separate layers.
NM_SETTING_NAME is also a GObject property, but it's
not supposed to be serialized to/from D-Bus. It also
is irrelevant for comparison.
Hence, it's operations are all NOPs. Make an explicit property type for
that case instead of checking the GParamSpec flags.
The "to_dbus_data" existed for namespacing the properties inside it.
However, such a struct adds overhead due to the alignment that it
enforces. We can share the memory needed for the bitfield by having
them beside each other.
The goal is to get rid of gprop_to_dbus_fcn() uses.
Note that there is a change in behavior. The "dns" GPtrArray in
NMSettingIPConfig is never NULL (the default of the boxed property),
thus the previous code always serialized the property, even the
empty list.
Now, empty dns properties are omitted from D-Bus.
Also, there is another change in behavior: nm_utils_ip4_dns_to_variant()
will now skip over strings that are not valid IPv4 addresses.
Previously, it would have added 0.0.0.0 (or some undefined address).
The goal is to get rid of gprop_to_dbus_fcn() uses.
Note that there is a change in behavior. The "dns" GPtrArray in
NMSettingIPConfig is never NULL (the default of the boxed property),
thus the previous code always serialized the property, even the
empty list.
Now, empty dns properties are omitted from D-Bus.
All settings have a "name" property. Their compare_fcn() is not interesting
and was already previously ignored. But we should not special handle it via
_nm_setting_property_compare_fcn_default().
So far, we only have NMSettingClass.compare_property() hook.
The ugliness is that this hook is per-setting, when basically
all implementations only compare one property.
It feels cleaner to have a per-property hook and call that consistently.
In step one, we give all properties (the same) compare_fcn() implementation,
which delegates to the existing NMSettingClass.compare_property().
In a second step, this will be untangled.
There is one problem with this approach: NMSettInfoPropertType grows by
one pointer size, and we have potentially many such types. That should
be addressed by unifying types in the future.
Various NMSetting API would accept a property_idx parameter. Together
with the NMSettInfoSetting instance, this was useful to find the actual
NMSettInfoProperty instance.
The idea was, to provide the most of the functionality. That is, if you
might need the property_idx too, you had it -- after all, the
property_info you could lookup yourself.
However,
- literally zero users care about the property_idx. The care about
the property_info.
- if the user really, really required the property_idx, then it
is a given that it can be easily computed by
(property_info - sett_info->property_infos)
We encode the default value "direct" properties in the GParamSpec.
But we also avoid CONSTRUCT properties, because they have an overhead
and they are generally odd for the settings.
So up to now, it was cumbersome to explicitly set the default value,
but it was also error prone.
Avoid that by always initializing the default value for our "direct"
properties.
And as example, implement NMSettingVrf.table this way. This also
makes all properties of NMSettingVrf implemened as "direct" properties,
and we can drop the explicit getter/setters.
If all settings would be strictly be implemented as "direct" properties,
we could call this from NMSetting.finalize() and be done with it.
As it is, for now we cannot, so it's still cumbersome.
For our property meta data handling we require that all the meta data is
associated with one GType.
NMSettingIPConfig is a parent class of NMSettingIP[46]Config. Note that
we already have _nm_sett_info_property_override_create_array_ip_config()
because the meta data must be initialized together at one place.
We will require that we can find the offset for properties based on one
offset per type. That is cumbersome, if NMSettingIPConfigPrivate is
private itself.
Simplify that, by internally sharing NMSettingIPConfigPrivate and let
the subclasses embed the private data in their own private data.
Optimally we would simply embed the private struct as field into
NMSettingIPConfig. But that would be an ABI change as that struct
was public before 1.32. Don't change ABI for now, so we have to
awkwardly place it into the subclasses private data.
Introduce a new mechanism for how to handle properties generically.
We have NMSettInfoSetting, NMSettInfoProperty and NMSettInfoPropertType
with meta data about settings and their properties.
For example, we have a simple boolean property. Then (usually) we have a
boolean GParamSpec, and a plain boolean field in the NMSetting's private
data. We need very little to get (and convert to keyfile, GVariant),
set (from keyfile, GVariant) and compare this property.
All we need to know, is the GParamSpec and the offset of the bool field.
Introduce a new mechanism for that, and as example implement
NM_SETTING_CONNECTION_AUTOCONNECT property this way.
Note that this patch only changes the to_dbus_fcn() for the boolean
property. But this opens up all kind of further improvements.
What we eventually also can do is replace GObjectClass.get_property()
with a generic variant, that knows how to get and set the property.
NMSetting instances either have no private data, they use
g_type_add_class_private(), or they embed the private data in the
NMSetting struct.
In all cases, we can find the private data at a fixed offset. Track that
offset in the NMSettInfoSetting meta data.
This will be useful, because properties really are stored in simple
fields, like a boolean property can be stored in a "bool" field. We will
extend the property meta data to track the offset of this property
field, but we also need to know where the offset starts.
