[PATCH v3 1/2] enumerate supported VPN protocols via openconnect_get_supported_protocols()

[Daniel Lenski]

On Thu, Feb 23, 2017 at 2:54 AM, David Woodhouse <dwmw2 at infradead.org> wrote:
>
> This mostly looks good; thanks. And apologies for delayed response.
>
> On Sun, 2017-01-15 at 13:40 -0800, Daniel Lenski wrote:
> >
> > +#define OC_PROTO_PROXY (1<<0)
> > +#define OC_PROTO_CSD   (1<<1)
> > +#define OC_PROTO_AUTH_CERT     (1<<2)
> > +#define OC_PROTO_AUTH_OTP      (1<<4)
> > +#define OC_PROTO_AUTH_STOKEN   (1<<8)
>
> That's 0x01, 0x02, 0x04, 0x10, 0x100… and would we add 0x10000 next?
>
> Not what you meant to do, I suspect. :)

Indeed not. I'll fix that.

>
> The .description fields for the protocols probably want to be
> translatable.

Will do.

> And let's take a look at how this is handled in
> NetworkManager-openconnect. See the handling of 'supported_protocols'
> and the hard-coded strings in the _vt_impl_get_service_add_detail()
> function... I think we want to be providing both those "pretty name"
> and "description" strings, don't we?

This here, right?
https://git.gnome.org/browse/network-manager-openconnect/tree/properties/nm-openconnect-editor-plugin.c#n394

I'll add the pretty_name as well.

> Finally, in Windows we can't allocate and return memory and expect the
> caller to free it. If we it that way, we need to provide an
> openconnect_free_supported_protocols() function too.

Will do.

-Dan