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

Re: [Xen-devel] [PATCH v3 02/12] livepatch: Allow to override inter-modules buildid dependency



On 9/17/19 9:27 AM, Wieczorkiewicz, Pawel wrote:


On 16. Sep 2019, at 19:01, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx <mailto:ross.lagerwall@xxxxxxxxxx>> wrote:

On 9/16/19 11:59 AM, Pawel Wieczorkiewicz wrote:
snip
+/*
+ * Parse user provided action flags.
+ * This function expects to only receive an array of input parameters being flags. + * Expected action is specified via idx paramater (index of flag_options[]).
+ */
+static int get_flags(int argc, char *argv[], unsigned int idx, uint64_t *flags)
+{
+    int i, j;
+
+    if ( !flags || idx >= ARRAY_SIZE(flag_options) )
+        return -1;
+
+    *flags = 0;
+    for ( i = 0; i < argc; i++ )
+    {
+        for ( j = 0; j < ARRAY_SIZE(flag_options[idx]); j++ )
+        {
+            if ( !flag_options[idx][j].name )
+                goto error;
+
+            if ( !strcmp(flag_options[idx][j].name, argv[i]) )
+            {
+                *flags |= flag_options[idx][j].flag;
+                break;
+            }
+        }
+
+        if ( j == ARRAY_SIZE(flag_options[idx]) )
+            goto error;
+    }
+
+    return 0;
+error:
+    fprintf(stderr, "Unsupported flag: %s.\n", argv[i]);
+    errno = EINVAL;
+    return errno;
+}

You return -1 above but +ve errno here. Please make it consistent.

Well, I understood from the code of the file (e.g. action_func()) that the -1 value indicates a unexpected runtime error (negative val).
Whereas, positive errno values are expected error to be dealt with.

So:
<0 - fatal errors
0 - ok
 >0 - errors to be handled

Could you confirm please that I should make get_flags() return only positive errors?

From what I can see, the only positive errors that are "handled" are EAGAIN and EBUSY to report EXIT_TIMEOUT and the mixture of returning -1 and positive errno is a bug. But it's fine to leave it as is for now so it.


Also, you don't need to set errno if returning the actual error.


Honestly, I just copied the code from get_name() and wanted to the get_flags() to follow similar pattern.

Sure.

--
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.