Hi,
Thanks for the quick feedback.
On Fri, Feb 19, 2016 at 6:37 AM, Gert Doering <g...@greenie.muc.de> wrote:
> Hi,
>
> On Fri, Feb 19, 2016 at 01:51:02AM -0500, Selva Nair wrote:
> > The pacth is in the next email.
> >
> > This is not yet tested extensively, but has passed some quick tests.
> > A simple approach of parsing the options string is used instead of
> passing structs:
> > the latter will break the GUI everytime a new option is added to the
> white-list.
> >
> > Current white-list is just what the GUI needs, but its easy to extend.
>
> Re-thinking the original argument, I agree that this is easier than
> a fixed structure (if only because you'd need to ensure that GUI and
> service are talking the same structure version...).
>
> I like what your patch is doing - it's more than "just the whitelist"
> but also the administrative restrictions ("if you are not administrator
> or part of the OpenVPN Admin group, only configs from a well-known
> directory, otherwise, do what you want").
>
And the admin (or at install time) could even change the "wheel" group name
to "Users" and bless every user too.
>
> From a C perspective, the code looks good to me.
>
> I'm a bit reluctant to ACK & merge it, I'd like a few more eyes look
> on this, from two angles
>
> - is this feature-wise the way we want to go? It "works for me"
>
> - are the windows bits of the code change sane? String manipulation and
> wide strings and that stuff is something I have little experience with.
>
String conversions are indeed a pain. Could eliminate them if the code is
guaranteed to be compiled with -municode -D_UNICODE. Does anyone build a
non-unicode version on windows these days?
The whole of the service code will then need only one string conversion --
for passing the management password to stdin of openvpn.
Selva
