Re: [PATCH v3 1/7] util: introduce a parser for kernel cmdline arguments
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(a)linux.ibm.com>
> Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
Reviewed-by: Erik Skultety <eskultet(a)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, ¶m, &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