[strongSwan-dev] configurable SPI-range in charon

[Martin Willi]

Hi Thomas,

Thanks for your patch-set. Some comments:

> +       this->public.get_char = (char(*)(settings_t*, char *key, char def, ...))get_char;
> +       this->public.get_int8 = (int8_t(*)(settings_t*, char *key, int8_t def, ...))get_int8;
> +       this->public.get_uint8 = (u_int8_t(*)(settings_t*, char *key, u_int8_t def, ...))get_uint8;
> +       this->public.get_int16 = (int16_t(*)(settings_t*, char *key, int16_t def, ...))get_int16;
> +       this->public.get_uint16 = (u_int16_t(*)(settings_t*, char *key, u_int16_t def, ...))get_uint16;
> +       this->public.get_int32 = (int(*)(settings_t*, char *key, int def, ...))get_int;
> +       this->public.get_uint32 = (u_int32_t(*)(settings_t*, char *key, u_int32_t def, ...))get_uint;
> +       this->public.get_uint = (u_int32_t(*)(settings_t*, char *key, u_int32_t def, ...))get_uint;
> +       this->public.get_int64 = (int64_t(*)(settings_t*, char *key, int64_t def, ...))get_int64;
> +       this->public.get_uint64 = (u_int64_t(*)(settings_t*, char *key, u_int64_t def, ...))get_uint64; 

Do we really need this full set of methods? I fully agree to a
get_uint(), but the 8/16/32 getters actually just check the size of the
resulting value. The caller could check the sanity of the values where
appropriate, as you do it in the kernel_interface anyway at some degree.
The 64 getters might make sense, but are they used anywhere yet?

I'd like to keep the settings_t interface as simple as possible. I have
some plans to modularize the settings layer (to e.g. store the values in
a SQL database). Having a too complex interface makes implementation of
such a backend more and more complex.

Regards
Martin