[PATCH v3 1/7] util: introduce a parser for kernel cmdline arguments

Paulo de Rezende Pinatti ppinatti at linux.ibm.com
Mon Jun 15 14:49:10 UTC 2020



On 15/06/20 16:16, Erik Skultety wrote:
> On Mon, Jun 15, 2020 at 10:28:06AM +0200, Paulo de Rezende Pinatti wrote:
>> Introduce two utility functions to parse a kernel command
>> line string according to the kernel code parsing rules in
>> order to enable the caller to perform operations such as
>> verifying whether certain argument=value combinations are
>> present or retrieving an argument's value.
>>
>> Signed-off-by: Paulo de Rezende Pinatti <ppinatti at linux.ibm.com>
>> Signed-off-by: Boris Fiuczynski <fiuczy at linux.ibm.com>
> 
> Reviewed-by: Erik Skultety <eskultet at redhat.com>
> 
> The code is fine, I just spotted last minute codestyle nitpick that I didn't
> pay attention to previously, so if you're okay with the snippet below, I'll
> squash it in before merging:

I just think the description of the flag SEARCH_FIRST might not be clear 
enough without the "no matter the order" text, as it might suggest it 
just checks the first occurrence of the parameter and stops there (as 
opposed to the flag SEARCH_LAST which checks only the last occurrence). 
I suggest using "match any occurrence of pattern" to avoid confusion.

Everything else (including the changes in the other patches) looks good 
to me.


> -/* Kernel cmdline match and comparison strategy for arg=value pairs */
>   typedef enum {
> -    VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1, /* substring comparison of argument values > -    VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2, /* strict string comparison 
of argument values */
> -    VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4, /* match first occurrence of pattern */
> -    VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8, /* match last occurrence of pattern */
> +    VIR_KERNEL_CMDLINE_FLAGS_CMP_PREFIX = 1,
> +    VIR_KERNEL_CMDLINE_FLAGS_CMP_EQ = 2,
> +    VIR_KERNEL_CMDLINE_FLAGS_SEARCH_FIRST = 4,
> +    VIR_KERNEL_CMDLINE_FLAGS_SEARCH_LAST = 8,
>   } virKernelCmdlineFlags;
> 
>   const char *virKernelCmdlineNextParam(const char *cmdline,
> diff --git a/tests/utiltest.c b/tests/utiltest.c
> index 2bff7859dc..e6c1bb1050 100644
> --- a/tests/utiltest.c
> +++ b/tests/utiltest.c
> @@ -286,12 +286,14 @@ static struct testKernelCmdlineNextParamData kEntries[] = {
>   static int
>   testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED)
>   {
> +    char *param = NULL;
> +    char *val = NULL;
>       const char *next;
>       size_t i;
> 
>       for (i = 0; i < G_N_ELEMENTS(kEntries); ++i) {
> -        g_autofree char * param = NULL;
> -        g_autofree char * val = NULL;
> +        VIR_FREE(param);
> +        VIR_FREE(val);
> 
>           next = virKernelCmdlineNextParam(kEntries[i].cmdline, &param, &val);
> 
> @@ -306,10 +308,16 @@ testKernelCmdlineNextParam(const void *data G_GNUC_UNUSED)
>               VIR_TEST_DEBUG("Expect next [%s]", kEntries[i].next);
>               VIR_TEST_DEBUG("Actual next [%s]", next);
> 
> +            VIR_FREE(param);
> +            VIR_FREE(val);
> +
>               return -1;
>           }
>       }
> 
> +    VIR_FREE(param);
> +    VIR_FREE(val);
> +
>       return 0;
>   }
> 
> 

-- 
Best regards,

Paulo de Rezende Pinatti




More information about the libvir-list mailing list