GNOME Bugzilla – Bug 734230
[review] refactor logging in NMDevice subclasses [th/bgo734230_device_logging]
Last modified: 2014-09-19 18:57:15 UTC
use the new logging macros _LOG[DIWE]() also for subclasses of NMDevice
branch: th/bgo734230_device_logging
What not just: + (self) ? str_if_set (nm_device_get_iface ((NMDevice *) (self)), "(null)") : "(none)" \ instead of having self_to_device() which just does the same thing? I would say it's a programmer error if that is not true everywhere we use these macros, plus you've added NM_IS_DEVICE() to nm_device_get_iface() so that will return NULL if for some reason we introduced a bug.
(In reply to comment #2) > What not just: > > + (self) ? str_if_set (nm_device_get_iface ((NMDevice *) > (self)), "(null)") : "(none)" \ > > instead of having self_to_device() which just does the same thing? I would say > it's a programmer error if that is not true everywhere we use these macros, > plus you've added NM_IS_DEVICE() to nm_device_get_iface() so that will return > NULL if for some reason we introduced a bug. At least while porting over to the new macros it was useful because I got a compile error if the "self" pointer was of an unexpected type. Sometimes @self was the parent type NMDevice* or even "NMModem *self". I could drop it, but I slightly prefer the additional compile time check. ??
I don't particularly like it, but I do see the utility of having it a compile-time check instead of a runtime check. However, could we tighten that up with: #define DECLARE_LOG_SELF(t) \ inline static NMDevice * \ _self_to_device (t *self) \ { \ return (NMDevice *) self; \ } \ and then in the files themselves after #include "nm-device-logging.h" we'd do: DECLARE_LOG_SELF(NMDeviceTeam) which would look cleaner to my eyes and be less code too.
Yeah, what's there now with _LOG_DECLARE_SELF() looks fine to me.
merged as http://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=86ab915f6af452a38b7d112d63c4989f052024c2