Hi Beniamino,
Thanks for your review!
On Wed, Jun 20, 2018 at 10:39:00AM +0200, Beniamino Galvani wrote:
> > +typedef struct {
> > + NMIPAddr ip;
> > + guint8 family;
> > + guint8 cidr;
>
> Just a nitpick, feel free to ignore, but perhaps 'mask' would be a
> better name than 'cidr'.
Sure, if it parses better I'm all for it :)
> > [...]
> > @@ -7103,6 +7416,8 @@ constructed (GObject *_object)
> > g_assert (!nle);
> > _LOGD ("Generic netlink socket established: port=%u, fd=%d",
> > nl_socket_get_local_port (priv->genlh), nl_socket_get_fd (priv->genlh));
> >
> > + priv->wireguard_family_id = _support_genl_family (priv->genlh,
> > "wireguard");
> > +
>
> Since the genl family id is already determined when needed (i.e. in
> wireguard_get_link_properties()), I don't think we need to do it here
> too.
Yeah you're right, I'll drop it.
> > +void
> > +nm_platform_lnk_wireguard_hash_update (const NMPlatformLnkWireguard *obj,
> > NMHashState *h)
> > +{
> > + nm_hash_update_vals (h,
> > + obj->private_key,
> > + obj->public_key,
> > + obj->listen_port,
> > + obj->fwmark);
> > +}
>
> This gives the following compile error here:
>
> CC src/platform/src_libNetworkManagerBase_la-nm-platform.lo
> In file included from ./shared/nm-default.h:292,
> from src/platform/nm-platform.c:21:
> src/platform/nm-platform.c: In function
> ‘nm_platform_lnk_wireguard_hash_update’:
> ./shared/nm-utils/nm-hash-utils.h:121:57: error: initialization of ‘unsigned
> char’ from ‘const guint8 *’ {aka ‘const unsigned char *’} makes integer from
> pointer without a cast [-Werror=int-conversion]
> #define _NM_HASH_COMBINE_VALS_val_x_4( y, ...) ._v4 = (y),
> _NM_HASH_COMBINE_VALS_val_x_3 (__VA_ARGS__)
>
> because private and public keys are guint8[]; I think you should use
> nm_hash_update() for them.
Oof. These were correctly using nm_hash_update in v2 too :/
Will fix.
(Reviewing my build settings now, it seems I lost -Werror in the process
of trying out meson, and ended up missing this warning.)
> The rest LGTM, thanks.
>
> Beniamino
Thank you!
_______________________________________________
networkmanager-list mailing list
[email protected]
https://mail.gnome.org/mailman/listinfo/networkmanager-list
