[libvirt] [PATCH V2] virconf: properly set the end of content

[Jim Fehlig]

Opps, sorry for missing the 'PATCH V2' subject-prefix.

Regards,
Jim

On 11/08/2017 04:01 PM, Jim Fehlig wrote:
> There was a recent report of the xen-xl converter not handling
> config files missing an ending newline
> 
> https://www.redhat.com/archives/libvir-list/2017-October/msg01353.html
> 
> Commit 3cc2a9e0 fixed a similar problem when parsing content of a
> file but missed parsing in-memory content. But AFAICT, the better
> fix is to properly set the end of the content when initializing the
> virConfParserCtxt in virConfParse().
> 
> This commit reverts the part of 3cc2a9e0 that appends a newline to
> files missing it, and fixes setting the end of content when
> initializing virConfParserCtxt. A test is also added to check
> parsing in-memory content missing an ending newline.
> 
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
>   src/util/virconf.c  | 13 ++--------
>   tests/virconftest.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 71 insertions(+), 11 deletions(-)
> 
> diff --git a/src/util/virconf.c b/src/util/virconf.c
> index 39c2bd917..a82a509ca 100644
> --- a/src/util/virconf.c
> +++ b/src/util/virconf.c
> @@ -705,7 +705,7 @@ virConfParse(const char *filename, const char *content, int len,
>   
>       ctxt.filename = filename;
>       ctxt.base = ctxt.cur = content;
> -    ctxt.end = content + len - 1;
> +    ctxt.end = content + len;
>       ctxt.line = 1;
>   
>       ctxt.conf = virConfCreate(filename, flags);
> @@ -745,7 +745,7 @@ virConfReadFile(const char *filename, unsigned int flags)
>   {
>       char *content;
>       int len;
> -    virConfPtr conf = NULL;
> +    virConfPtr conf;
>   
>       VIR_DEBUG("filename=%s", NULLSTR(filename));
>   
> @@ -757,17 +757,8 @@ virConfReadFile(const char *filename, unsigned int flags)
>       if ((len = virFileReadAll(filename, MAX_CONFIG_FILE_SIZE, &content)) < 0)
>           return NULL;
>   
> -    if (len && len < MAX_CONFIG_FILE_SIZE && content[len - 1] != '\n') {
> -        VIR_DEBUG("appending newline to busted config file %s", filename);
> -        if (VIR_REALLOC_N(content, len + 2) < 0)
> -            goto cleanup;
> -        content[len++] = '\n';
> -        content[len] = '\0';
> -    }
> -
>       conf = virConfParse(filename, content, len, flags);
>   
> - cleanup:
>       VIR_FREE(content);
>   
>       return conf;
> diff --git a/tests/virconftest.c b/tests/virconftest.c
> index a8b18bae0..3cf0df3ac 100644
> --- a/tests/virconftest.c
> +++ b/tests/virconftest.c
> @@ -77,6 +77,72 @@ static int testConfRoundTrip(const void *opaque)
>   }
>   
>   
> +static int testConfMemoryNoNewline(const void *opaque ATTRIBUTE_UNUSED)
> +{
> +    const char *srcdata = \
> +        "ullong = '123456789'\n" \
> +        "string = 'foo'\n" \
> +        "uint = 12345";
> +
> +    virConfPtr conf = virConfReadString(srcdata, 0);
> +    int ret = -1;
> +    virConfValuePtr val;
> +    unsigned long long llvalue;
> +    char *str = NULL;
> +    int uintvalue;
> +
> +    if (!conf)
> +        return -1;
> +
> +    if (!(val = virConfGetValue(conf, "ullong")))
> +        goto cleanup;
> +
> +    if (val->type != VIR_CONF_STRING)
> +        goto cleanup;
> +
> +    if (virStrToLong_ull(val->str, NULL, 10, &llvalue) < 0)
> +        goto cleanup;
> +
> +    if (llvalue != 123456789) {
> +        fprintf(stderr, "Expected '123' got '%llu'\n", llvalue);
> +        goto cleanup;
> +    }
> +
> +    if (virConfGetValueType(conf, "string") !=
> +        VIR_CONF_STRING) {
> +        fprintf(stderr, "expected a string for 'string'\n");
> +        goto cleanup;
> +    }
> +
> +    if (virConfGetValueString(conf, "string", &str) < 0)
> +        goto cleanup;
> +
> +    if (STRNEQ_NULLABLE(str, "foo")) {
> +        fprintf(stderr, "Expected 'foo' got '%s'\n", str);
> +        goto cleanup;
> +    }
> +
> +    if (virConfGetValueType(conf, "uint") != VIR_CONF_ULLONG) {
> +        fprintf(stderr, "expected an unsigned long for 'uint'\n");
> +        goto cleanup;
> +    }
> +
> +    if (virConfGetValueInt(conf, "uint", &uintvalue) < 0)
> +        goto cleanup;
> +
> +    if (uintvalue != 12345) {
> +        fprintf(stderr, "Expected 12345 got %ud\n", uintvalue);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(str);
> +    virConfFree(conf);
> +    return ret;
> +}
> +
> +
>   static int testConfParseInt(const void *opaque ATTRIBUTE_UNUSED)
>   {
>       const char *srcdata = \
> @@ -414,6 +480,9 @@ mymain(void)
>       if (virTestRun("no-newline", testConfRoundTrip, "no-newline") < 0)
>           ret = -1;
>   
> +    if (virTestRun("memory-no-newline", testConfMemoryNoNewline, NULL) < 0)
> +        ret = -1;
> +
>       if (virTestRun("int", testConfParseInt, NULL) < 0)
>           ret = -1;
>   
>