Re: [Openvpn-devel] [PATCH 2/7] compat-mode: allow user to specify version to be compatible with

[Greg C. <gc...@mo...>]

On Sat, Sep 4, 2021 at 9:57 AM Antonio Quartulli <a...@un...> wrote:

> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 0d6b85cf..4d971a56 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -3125,6 +3125,29 @@ options_postprocess_cipher(struct options *o)
>      }
>  }
>
> +/**
> + * Returns if we want 'backwards-compatibility' up to (but not included) a
> + * certain version
> + *
> + * @param version   the oldest version that does not compatibility
>

I feel like you dropped a word in here ^^

> + *                  e.g. 20400 for all versions < 2.4.0
> + * @return          whether compatibility should be enabled
> + */
> +static bool
> +need_compatibility_before(const struct options *o, int version)
>

I feel like you want to do `unsigned int` on your version parameter here
(why would a version go negative?) ...



> +{
> +    return o->backwards_compatible != 0 && o->backwards_compatible <
> version;
> +}
> +
> +/**
> + * Changes default values so that OpenVPN can be compatible with the user
> + * specified version
> + */
> +static void
> +options_set_backwards_compatible_options(struct options *o)
> +{
> +}
> +
>  static void
>  options_postprocess_mutate(struct options *o)
>  {
> @@ -3137,6 +3160,8 @@ options_postprocess_mutate(struct options *o)
>      helper_keepalive(o);
>      helper_tcp_nodelay(o);
>
> +    options_set_backwards_compatible_options(o);
> +
>      options_postprocess_cipher(o);
>      options_postprocess_mutate_invariant(o);
>
> @@ -6698,6 +6723,18 @@ add_option(struct options *options,
>              setenv_str(es, p[1], p[2] ? p[2] : "");
>          }
>      }
> +    else if (streq(p[0], "compat-mode") && p[1] && !p[3])
> +    {
> +        unsigned int major, minor, patch;
> +        if (!(sscanf(p[1], "%u.%u.%u", &major, &minor, &patch) == 3))
> +        {
> +            msg(msglevel, "cannot parse version number for --compat-mode:
> %s",
> +                p[1]);
> +            goto err;
> +        }
> +
> +        options->backwards_compatible = major * 10000 + minor * 100 +
> patch;
> +    }
>      else if (streq(p[0], "setenv-safe") && p[1] && !p[3])
>      {
>          VERIFY_PERMISSION(OPT_P_SETENV);
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index b0e40cb7..98c21a2a 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -225,6 +225,10 @@ struct options
>
>      /* enable forward compatibility for post-2.1 features */
>      bool forward_compatible;
> +    /** What version we should try to be compatible with as major * 10000
> +
> +      * minor * 100 + patch, e.g. 2.4.7 => 20407 */
> +    unsigned int backwards_compatible;
>

... doubly so because you use `unsigned int` (imo rightly) here.

> +
>      /* list of options that should be ignored even if unknown */
>      const char **ignore_unknown_option;
>
> --
> 2.32.0
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Ope...@li...
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>