Re: [libvirt] [PATCH] virconf: properly set the end of content
On Wed, Nov 08, 2017 at 04:01:48PM -0700, 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 > --- > src/util/virconf.c | 13 ++ > tests/virconftest.c | 69 > + > 2 files changed, 71 insertions(+), 11 deletions(-) Reviewed-by: Daniel P. Berrange Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virconf: properly set the end of content
On 11/08/2017 01:56 AM, Daniel P. Berrange wrote: On Tue, Nov 07, 2017 at 03:37:46PM -0700, 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. Signed-off-by: Jim Fehlig --- src/util/virconf.c | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) Could you try to extend tests/virconftest.c to cover in-memory content for this newline scenario too. I cooked up some content that failed without this change and passes with it. V2 sent https://www.redhat.com/archives/libvir-list/2017-November/msg00340.html Regards, Jim -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virconf: properly set the end of content
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
---
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;
--
2.14.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virconf: properly set the end of content
On Tue, Nov 07, 2017 at 03:37:46PM -0700, 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.
>
> Signed-off-by: Jim Fehlig
> ---
> src/util/virconf.c | 13 ++---
> 1 file changed, 2 insertions(+), 11 deletions(-)
Could you try to extend tests/virconftest.c to cover in-memory
content for this newline scenario too.
>
> 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;
> --
> 2.14.2
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virconf: properly set the end of content
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.
Signed-off-by: Jim Fehlig
---
src/util/virconf.c | 13 ++---
1 file changed, 2 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;
--
2.14.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
