On 16. Sep 2019, at 19:01, Ross Lagerwall
<ross.lagerw...@citrix.com<mailto:ross.lagerw...@citrix.com>> 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?
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.
(The error handling in this file looks fairly inconsistent anyway but let's not
make it worse.)
+
/* The hypervisor timeout for the live patching operation is 30 msec,
* but it could take some time for the operation to start, so wait twice
* that period. */
@@ -291,8 +357,9 @@ int action_func(int argc, char *argv[], unsigned int idx)
char name[XEN_LIVEPATCH_NAME_SIZE];
int rc;
xen_livepatch_status_t status;
+ uint64_t flags;
- if ( argc != 1 )
+ if ( argc < 1 )
{
show_help();
return -1;
@@ -301,7 +368,10 @@ int action_func(int argc, char *argv[], unsigned int idx)
if ( idx >= ARRAY_SIZE(action_options) )
return -1;
- if ( get_name(argc, argv, name) )
+ if ( get_name(argc--, argv++, name) )
+ return EINVAL;
+
+ if ( get_flags(argc, argv, idx, &flags) )
return EINVAL;
/* Check initial status. */
@@ -332,7 +402,7 @@ int action_func(int argc, char *argv[], unsigned int idx)
if ( action_options[idx].allow & status.state )
{
printf("%s %s... ", action_options[idx].verb, name);
- rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS);
+ rc = action_options[idx].function(xch, name, HYPERVISOR_TIMEOUT_NS,
flags);
if ( rc )
{
int saved_errno = errno;
@@ -394,17 +464,23 @@ int action_func(int argc, char *argv[], unsigned int idx)
static int load_func(int argc, char *argv[])
{
- int rc;
- char *new_argv[2];
- char *path, *name, *lastdot;
+ int i, rc = ENOMEM;
+ char *upload_argv[2];
+ char **apply_argv, *path, *name, *lastdot;
- if ( argc != 1 )
+ if ( argc < 1 )
{
show_help();
return -1;
}
+
+ /* apply action has <id> [flags] input requirement, which must be
constructed */
+ apply_argv = (char **) malloc(argc * sizeof(*apply_argv));
+ if ( !apply_argv )
+ return rc;
+
/* <file> */
- new_argv[1] = argv[0];
+ upload_argv[1] = argv[0];
/* Synthesize the <id> */
path = strdup(argv[0]);
@@ -413,16 +489,23 @@ static int load_func(int argc, char *argv[])
lastdot = strrchr(name, '.');
if ( lastdot != NULL )
*lastdot = '\0';
- new_argv[0] = name;
+ upload_argv[0] = name;
+ apply_argv[0] = name;
- rc = upload_func(2 /* <id> <file> */, new_argv);
+ /* Fill in all user provided flags */
+ for ( i = 0; i < argc - 1; i++ )
+ apply_argv[i + 1] = argv[i + 1];
Wouldn't this make the loop body simpler? i = 1; i < argc;
ACK. It would indeed.
Or alternatively, just a straight memcpy().
--
Ross Lagerwall
Best Regards,
Pawel Wieczorkiewicz
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
