Hi, On 05/07/18 01:53, Steffan Karger wrote: > To avoid having to include misc.c - which is a dependency mess - in the > tls-crypt unit tests, move the command execution helper functions to a new > run_command.c module. >
one thing less into misc.c :-)
After having witnessed the dependency mess in the unit-test, I am
definitely "pro"-this-patch as it simplifies quite a bit the dep-hell.
> While at it, abstract away the script_security global variable.
I like the idea of getting rid of a process-wide global variable,
however I think that script_security should simply be a member of the
options object or of one of the contexts, rather than just hanging on
its own.
However, I'd postpone that to another patch. Your change here is already
a step forward.
>
> Signed-off-by: Steffan Karger <steffan.kar...@fox-it.com>
> ---
[CUT]
> +/* contains an SSEC_x value defined in platform.h */
> +static int script_security_level = SSEC_BUILT_IN; /* GLOBAL */
Maybe this "GLOBAL" thing could be removed as this variable now exists
only within this file?
> +
> +int script_security(void)
> +{
> + return script_security_level;
> +}
> +
> +void script_security_set(int level)
> +{
> + script_security_level = level;
> +}
> +
[CUT]
Other than that little nitpick, the patch looks good.
Checked with "git show --color-moved" and I could verify that the code
has only been moved (slightly adjusted to avoid long lines) and that the
access to the security_script variable has been substituted with
getter/setter functions.
So, with or without the nitpick:
Acked-by: Antonio Quartulli <a...@unstable.cc>
Cheers,
--
Antonio Quartulli
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
