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


_______________________________________________
Dev mailing list
[email protected]
https://lists.strongswan.org/mailman/listinfo/dev

Reply via email to