Re: [Xen-devel] [PATCH v3 1/5] libxl: allow libxl__exec to take a parameter containing the env variables

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[Xen-devel] [PATCH v3 1/5] libxl: allow libxl__exec to 
take a parameter containing the env variables"):
> Add another parameter to libxl__exec call that contains the
> environment variables to use when performing the execvp call.

This looks OK to me.  However, shouldn't this be const char** ?

>      struct termios termattr;
> +    char *env[] = {"TERM", "vt100", NULL};

We don't compile with -Wwrite-strings but if we did this would
trigger.  Also it needs spaces inside { } I think.

> +    if (env != NULL) {
> +        for (int i = 0; env[i] != NULL && env[i+1] != NULL; i += 2) {
> +            setenv(env[i], env[i+1], 1);

setenv can fail.  If it does you should probably check errno and
probably call libxl__alloc_failed.

> +/*
> + * env should be passed using the following format,
> + *
> + * env[0]: name of env variable
> + * env[1]: value of env variable

You need to mention that it may have more than one setting!

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel