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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxxxxxx>
Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxx>

Reviewed-by: Erik Skultety <eskultet@xxxxxxxxxx>

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux