Re: [PATCH 06/12] mkfs: create get/set functions for opts table

Jan Tulak
2017-04-25T08:11:28Z

On Tue, Apr 25, 2017 at 5:40 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Sun, Apr 23, 2017 at 08:54:57PM +0200, Jan Tulak wrote:
>> Add functions that can be used to get/set values to opts table.
>>
>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>>
>> ---
>> mkfs/xfs_mkfs.c | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index c2ffd91..4caf93c 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -786,6 +786,38 @@ get_conf_raw(int opt, int subopt)
>> return opts[opt].subopt_params[subopt].raw_input;
>> }
>>
>> +static uint64_t getnum(const char *str, struct opt_params *opts, int index);
>
> Why not just move getnum() above here. Forward declarations IMHO should not be
> needed unless we have odd inclusion issues and I don't think that's the case
> here ?

Getnum requires set_conf_raw and illegal_option, but it seems that
they could be moved too. I think there is no circle dependency, I just
didn't want to move so many lines when I can do one declaration. But
if the declaration is an issue, I can shuffle the ~80 lines the three
functions takes.

Jan

>
>> +
>> +/*
>> + * Get and set values to the opts table.
>> + */
>> +static inline uint64_t
>> +get_conf_val(int opt, int subopt)
>> +{
>> + return opts[opt].subopt_params[subopt].value;
>> +}
>> +
>> +static void
>> +set_conf_val(int opt, int subopt, uint64_t val)
>> +{
>> + struct subopt_param *sp = &opts[opt].subopt_params[subopt];
>> +
>> + sp->value = val;
>> +}
>> +
>> +/*
>> + * A wrapper for getnum and set_conf_val.
>> + */
>> +static inline uint64_t
>> +parse_conf_val(int opt, int subopt, char *value)
>> +{
>> + uint64_t num = getnum(value, &opts[opt], subopt);
>> +
>> + set_conf_val(opt, subopt, num);
>> + return num;
>> +}
>
> Very nice thanks!
>
> Luis

--
Jan Tulak
jtulak@redhat.com / jan@tulak.me