On Mon, Dec 12, 2022 at 12:38:41PM +0100, Arne Schwabe wrote:
> Am 27.11.22 um 15:25 schrieb Frank Lichtenheld:
> > That makes it possible to remove several preprocessor
> > directives which is a good thing. The cost should be
> > negligible.
> 
> Acked-By: Arne Schwabe <a...@rfc2549.org>
> 
> In general the commit looks good but I would like to improve the comment for
> the management flags (see below).
> 
> 
> > diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> > index 68ad0cac..6f4b1f4a 100644
> > --- a/src/openvpn/options.h
> > +++ b/src/openvpn/options.h
> > @@ -438,10 +438,12 @@ struct options
> >       const char *management_client_user;
> >       const char *management_client_group;
> > -    /* Mask of MF_ values of manage.h */
> > -    unsigned int management_flags;
> >       const char *management_certificate;
> >   #endif
> > +    /* Mask of MF_ values of manage.h
> > +     * Always available to simplify options.c
> > +     */
> > +    unsigned int management_flags;
> 
> This feels a bit odd as a comment. I would expect this in the commit or
> something like "even available when management support is not available" but
> this comment feels a bit too "temporary".

How is this "temporary"? It does not describe the change, it actually states
why this is handled specially. Your proposal just drops the justification, then
the comment becomes useless IMHO.

Would you prefer "Always available to simplify option verification code"?

Regards,
-- 
  Frank Lichtenheld


_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to