[libvirt] [PATCH 0/2] patch pings
These are random patches in my tree that I've previously posted with no review, but which I think are useful for inclusion in 0.9.8. Eric Blake (2): command: handle empty buffer argument correctly build: require more tools from maintainers bootstrap.conf | 15 +++++++++++++++ src/util/command.c | 13 ++++++++++++- tests/commanddata/test9.log | 4 +++- tests/commandtest.c | 13 ++++++++++++- 4 files changed, 42 insertions(+), 3 deletions(-) -- 1.7.7.3
virBufferContentAndReset (intentionally) returns NULL for a buffer with no content, but it is feasible to invoke a command with an explicit empty string. * src/util/command.c (virCommandAddEnvBuffer): Reject empty string. (virCommandAddArgBuffer): Allow explicit empty argument. * tests/commandtest.c (test9): Test it. * tests/commanddata/test9.log: Adjust. --- src/util/command.c | 13 ++++++++++++- tests/commanddata/test9.log | 4 +++- tests/commandtest.c | 13 ++++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/util/command.c b/src/util/command.c index c3ce361..f5effdf 100644 --- a/src/util/command.c +++ b/src/util/command.c @@ -983,6 +983,10 @@ virCommandAddEnvBuffer(virCommandPtr cmd, virBufferPtr buf) virBufferFreeAndReset(buf); return; } + if (!virBufferUse(buf)) { + cmd->has_error = EINVAL; + return; + } cmd->env[cmd->nenv++] = virBufferContentAndReset(buf); } @@ -1092,7 +1096,14 @@ virCommandAddArgBuffer(virCommandPtr cmd, virBufferPtr buf) return; } - cmd->args[cmd->nargs++] = virBufferContentAndReset(buf); + cmd->args[cmd->nargs] = virBufferContentAndReset(buf); + if (!cmd->args[cmd->nargs]) + cmd->args[cmd->nargs] = strdup(""); + if (!cmd->args[cmd->nargs]) { + cmd->has_error = ENOMEM; + return; + } + cmd->nargs++; } diff --git a/tests/commanddata/test9.log b/tests/commanddata/test9.log index 2607530..3a93c19 100644 --- a/tests/commanddata/test9.log +++ b/tests/commanddata/test9.log @@ -2,8 +2,10 @@ ARG:-version ARG:-log=bar.log ARG:arg1 ARG:arg2 -ARG:arg3 +ARG: ARG:arg4 +ARG:arg5 +ARG:arg6 ENV:DISPLAY=:0.0 ENV:HOME=/home/test ENV:HOSTNAME=test diff --git a/tests/commandtest.c b/tests/commandtest.c index dd6c248..efc48fe 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -352,11 +352,22 @@ static int test9(const void *unused ATTRIBUTE_UNUSED) { virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); const char* const args[] = { "arg1", "arg2", NULL }; + virBuffer buf = VIR_BUFFER_INITIALIZER; virCommandAddArg(cmd, "-version"); virCommandAddArgPair(cmd, "-log", "bar.log"); virCommandAddArgSet(cmd, args); - virCommandAddArgList(cmd, "arg3", "arg4", NULL); + virCommandAddArgBuffer(cmd, &buf); + virBufferAddLit(&buf, "arg4"); + virCommandAddArgBuffer(cmd, &buf); + virCommandAddArgList(cmd, "arg5", "arg6", NULL); + + if (virBufferUse(&buf)) { + printf("Buffer not transferred\n"); + virBufferFreeAndReset(&buf); + virCommandFree(cmd); + return -1; + } if (virCommandRun(cmd, NULL) < 0) { virErrorPtr err = virGetLastError(); -- 1.7.7.3
On 12/02/2011 01:50 PM, Eric Blake wrote:
virBufferContentAndReset (intentionally) returns NULL for a buffer with no content, but it is feasible to invoke a command with an explicit empty string.
* src/util/command.c (virCommandAddEnvBuffer): Reject empty string. (virCommandAddArgBuffer): Allow explicit empty argument. * tests/commandtest.c (test9): Test it. * tests/commanddata/test9.log: Adjust. --- src/util/command.c | 13 ++++++++++++- tests/commanddata/test9.log | 4 +++- tests/commandtest.c | 13 ++++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-)
ACK
On 12/03/2011 02:18 PM, Stefan Berger wrote:
On 12/02/2011 01:50 PM, Eric Blake wrote:
virBufferContentAndReset (intentionally) returns NULL for a buffer with no content, but it is feasible to invoke a command with an explicit empty string.
* src/util/command.c (virCommandAddEnvBuffer): Reject empty string. (virCommandAddArgBuffer): Allow explicit empty argument. * tests/commandtest.c (test9): Test it. * tests/commanddata/test9.log: Adjust. --- src/util/command.c | 13 ++++++++++++- tests/commanddata/test9.log | 4 +++- tests/commandtest.c | 13 ++++++++++++- 3 files changed, 27 insertions(+), 3 deletions(-)
ACK
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
We want our tarballs to be complete - this means that any generated file that gets shipped as part of the tarball so that ordinary users don't have to rebuild it must be something that the maintainer can generate. There have been various reports of random build failures when using libvirt.git instead of a tarball, and often it is due to missing a maintainer-specific tool to produce one of these generated files. This patch raises the bar for what you must have installed to build libvirt.git, but does not impact what you can get away with for building tarballs. Note: It still remains possible to do a successful 'make dist' without these tools, when starting from a release tarball. * bootstrap.conf (buildreq): Add tools that maintainers need for a successful 'make dist' from a fresh git checkout. --- bootstrap.conf | 15 +++++++++++++++ 1 files changed, 15 insertions(+), 0 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 6498aba..a291590 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -181,6 +181,12 @@ ACLOCAL="$ACLOCAL -I m4" export ACLOCAL # Build prerequisites +# Note that some of these programs are only required for 'make dist' to +# succeed from a fresh git checkout; not all of these programs are +# required to run 'make dist' on a tarball. As a special case, we want +# to require the equivalent of the Fedora python-devel package, but +# RHEL 5 lacks the witness python-config package; we hack around that +# old environment below. buildreq="\ autoconf 2.59 automake 1.9.6 @@ -191,9 +197,18 @@ gzip - libtool - perl 5.5 pkg-config - +python-config - rpcgen - tar - +xmllint - +xsltproc - " +# You don't have to be on a system with rpm; rather, if you happen to +# be on RHEL 5, then this bypasses the bootstrap logic that probes for +# a working 'python-config --version'. +if `(rpm -q python-devel) >/dev/null 2>&1`; then + PYTHON_CONFIG=true +fi # Automake requires that ChangeLog exist. touch ChangeLog || exit 1 -- 1.7.7.3
On 12/02/2011 01:50 PM, Eric Blake wrote:
We want our tarballs to be complete - this means that any generated file that gets shipped as part of the tarball so that ordinary users don't have to rebuild it must be something that the maintainer can generate. There have been various reports of random build failures when using libvirt.git instead of a tarball, and often it is due to missing a maintainer-specific tool to produce one of these generated files. This patch raises the bar for what you must have installed to build libvirt.git, but does not impact what you can get away with for building tarballs.
Note: It still remains possible to do a successful 'make dist' without these tools, when starting from a release tarball.
* bootstrap.conf (buildreq): Add tools that maintainers need for a successful 'make dist' from a fresh git checkout. ---
ACK
On 12/03/2011 02:24 PM, Stefan Berger wrote:
On 12/02/2011 01:50 PM, Eric Blake wrote:
We want our tarballs to be complete - this means that any generated file that gets shipped as part of the tarball so that ordinary users don't have to rebuild it must be something that the maintainer can generate. There have been various reports of random build failures when using libvirt.git instead of a tarball, and often it is due to missing a maintainer-specific tool to produce one of these generated files. This patch raises the bar for what you must have installed to build libvirt.git, but does not impact what you can get away with for building tarballs.
Note: It still remains possible to do a successful 'make dist' without these tools, when starting from a release tarball.
* bootstrap.conf (buildreq): Add tools that maintainers need for a successful 'make dist' from a fresh git checkout. ---
ACK
Thanks; pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Stefan Berger