Re: [PATCH V1 17/32] util: env var helpers

[Steven Sistare <steven.sistare@oracle.com>]

On 9/11/2020 3:00 PM, Dr. David Alan Gilbert wrote:
> * Steve Sistare (steven.sistare@oracle.com) wrote:
>> Add functions for saving fd's and ram extents in the environment via
>> setenv, and for reading them back via getenv.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> Signed-off-by: Mark Kanda <mark.kanda@oracle.com>
> 
> This is an awful lot of env stuff - how about dumping
> all this stuff into a file and reloading it?

I don't think there will be significantly fewer lines if this is
re-written to use a file, and the existing code is very simple -- a few
easy to understand lines for each accessor.  Please skim env.c, you will
grok it in less time than it took to write our emails.  A file-based
version may be even  longer because the code would need to check for
malformed input, since the file contents could be changed outside qemu,
and we would need to provide an option for where the file is stored.

The env is nice because it eliminates a failure point -- the variables always 
get carried through to the post-exec process.  No lost or stale files.

- Steve

>> ---
>>  MAINTAINERS           |   7 +++
>>  include/qemu/cutils.h |   1 +
>>  include/qemu/env.h    |  31 ++++++++++++
>>  util/Makefile.objs    |   2 +-
>>  util/env.c            | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 172 insertions(+), 1 deletion(-)
>>  create mode 100644 include/qemu/env.h
>>  create mode 100644 util/env.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3395abd..8d377a7 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -3115,3 +3115,10 @@ Performance Tools and Tests
>>  M: Ahmed Karaman <ahmedkhaledkaraman@gmail.com>
>>  S: Maintained
>>  F: scripts/performance/
>> +
>> +Environment variable helpers
>> +M: Steve Sistare <steven.sistare@oracle.com>
>> +M: Mark Kanda <mark.kanda@oracle.com>
>> +S: Maintained
>> +F: include/qemu/env.h
>> +F: util/env.c
>> diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
>> index eb59852..d4c7d70 100644
>> --- a/include/qemu/cutils.h
>> +++ b/include/qemu/cutils.h
>> @@ -1,6 +1,7 @@
>>  #ifndef QEMU_CUTILS_H
>>  #define QEMU_CUTILS_H
>>  
>> +#include "qemu/env.h"
>>  /**
>>   * pstrcpy:
>>   * @buf: buffer to copy string into
>> diff --git a/include/qemu/env.h b/include/qemu/env.h
>> new file mode 100644
>> index 0000000..53cc121
>> --- /dev/null
>> +++ b/include/qemu/env.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Copyright (c) 2020 Oracle and/or its affiliates.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#ifndef QEMU_ENV_H
>> +#define QEMU_ENV_H
>> +
>> +#define FD_PREFIX "QEMU_FD_"
>> +#define ADDR_PREFIX "QEMU_ADDR_"
>> +#define LEN_PREFIX "QEMU_LEN_"
>> +#define BOOL_PREFIX "QEMU_BOOL_"
>> +
>> +typedef int (*walkenv_cb)(const char *name, const char *val, void *handle);
>> +
>> +bool getenv_ram(const char *name, void **addrp, size_t *lenp);
>> +void setenv_ram(const char *name, void *addr, size_t len);
>> +void unsetenv_ram(const char *name);
>> +int getenv_fd(const char *name);
>> +void setenv_fd(const char *name, int fd);
>> +void unsetenv_fd(const char *name);
>> +bool getenv_bool(const char *name);
>> +void setenv_bool(const char *name, bool val);
>> +void unsetenv_bool(const char *name);
>> +int walkenv(const char *prefix, walkenv_cb cb, void *handle);
>> +void printenv(void);
>> +
>> +#endif
>> diff --git a/util/Makefile.objs b/util/Makefile.objs
>> index cc5e371..d357932 100644
>> --- a/util/Makefile.objs
>> +++ b/util/Makefile.objs
>> @@ -1,4 +1,4 @@
>> -util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o
>> +util-obj-y = osdep.o cutils.o unicode.o qemu-timer-common.o env.o
>>  util-obj-$(call lnot,$(CONFIG_ATOMIC64)) += atomic64.o
>>  util-obj-$(CONFIG_POSIX) += aio-posix.o
>>  util-obj-$(CONFIG_POSIX) += fdmon-poll.o
>> diff --git a/util/env.c b/util/env.c
>> new file mode 100644
>> index 0000000..0cc4a9f
>> --- /dev/null
>> +++ b/util/env.c
>> @@ -0,0 +1,132 @@
>> +/*
>> + * Copyright (c) 2020 Oracle and/or its affiliates.
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "qemu/env.h"
>> +
>> +static uint64_t getenv_ulong(const char *prefix, const char *name, bool *found)
>> +{
>> +    char var[80], *val;
>> +    uint64_t res;
>> +
>> +    snprintf(var, sizeof(var), "%s%s", prefix, name);
>> +    val = getenv(var);
>> +    if (val) {
>> +        *found = true;
>> +        res = strtol(val, 0, 10);
>> +    } else {
>> +        *found = false;
>> +        res = 0;
>> +    }
>> +    return res;
>> +}
>> +
>> +static void setenv_ulong(const char *prefix, const char *name, uint64_t val)
>> +{
>> +    char var[80], val_str[80];
>> +    snprintf(var, sizeof(var), "%s%s", prefix, name);
>> +    snprintf(val_str, sizeof(val_str), "%"PRIu64, val);
>> +    setenv(var, val_str, 1);
>> +}
>> +
>> +static void unsetenv_ulong(const char *prefix, const char *name)
>> +{
>> +    char var[80];
>> +    snprintf(var, sizeof(var), "%s%s", prefix, name);
>> +    unsetenv(var);
>> +}
>> +
>> +bool getenv_ram(const char *name, void **addrp, size_t *lenp)
>> +{
>> +    bool found1, found2;
>> +    *addrp = (void *) getenv_ulong(ADDR_PREFIX, name, &found1);
>> +    *lenp = getenv_ulong(LEN_PREFIX, name, &found2);
>> +    assert(found1 == found2);
>> +    return found1;
>> +}
>> +
>> +void setenv_ram(const char *name, void *addr, size_t len)
>> +{
>> +    setenv_ulong(ADDR_PREFIX, name, (uint64_t)addr);
>> +    setenv_ulong(LEN_PREFIX, name, len);
>> +}
>> +
>> +void unsetenv_ram(const char *name)
>> +{
>> +    unsetenv_ulong(ADDR_PREFIX, name);
>> +    unsetenv_ulong(LEN_PREFIX, name);
>> +}
>> +
>> +int getenv_fd(const char *name)
>> +{
>> +    bool found;
>> +    int fd = getenv_ulong(FD_PREFIX, name, &found);
>> +    if (!found) {
>> +        fd = -1;
>> +    }
>> +    return fd;
>> +}
>> +
>> +void setenv_fd(const char *name, int fd)
>> +{
>> +    setenv_ulong(FD_PREFIX, name, fd);
>> +}
>> +
>> +void unsetenv_fd(const char *name)
>> +{
>> +    unsetenv_ulong(FD_PREFIX, name);
>> +}
>> +
>> +bool getenv_bool(const char *name)
>> +{
>> +    bool found;
>> +    bool val = getenv_ulong(BOOL_PREFIX, name, &found);
>> +    if (!found) {
>> +        val = -1;
>> +    }
>> +    return val;
>> +}
>> +
>> +void setenv_bool(const char *name, bool val)
>> +{
>> +    setenv_ulong(BOOL_PREFIX, name, val);
>> +}
>> +
>> +void unsetenv_bool(const char *name)
>> +{
>> +    unsetenv_ulong(BOOL_PREFIX, name);
>> +}
>> +
>> +int walkenv(const char *prefix, walkenv_cb cb, void *handle)
>> +{
>> +    char *str, name[128];
>> +    char **envp = environ;
>> +    size_t prefix_len = strlen(prefix);
>> +
>> +    while (*envp) {
>> +        str = *envp++;
>> +        if (!strncmp(str, prefix, prefix_len)) {
>> +            char *val = strchr(str, '=');
>> +            str += prefix_len;
>> +            strncpy(name, str, val - str);
>> +            name[val - str] = 0;
>> +            if (cb(name, val + 1, handle)) {
>> +                return 1;
>> +            }
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +void printenv(void)
>> +{
>> +    char **ptr = environ;
>> +    while (*ptr) {
>> +        puts(*ptr++);
>> +    }
>> +}
>> -- 
>> 1.8.3.1
>>
>>