Re: [libvirt] [PATCH 1/2] virCommand: document behavior on no output

2010-12-05 Thread Daniel Veillard
On Sun, Dec 05, 2010 at 12:55:51AM +0100, Matthias Bolte wrote:
> 2010/12/4 Justin Clift :
> > On 04/12/2010, at 8:28 AM, Eric Blake wrote:
> >> Option 1: This patch (all callers have to worry about NULL buffers,
> >> but checking for output is a simple pointer check).
> >>
> >> Option 2: Guarantee that outbuf/errbuf are allocated, even if to
> >> the empty string.  Caller always has to free the result, and
> >> empty output check requires checking if *outbuf=='\0'.
> >>
> >> Personally, I prefer option 2.  Thoughts?
> >
> > 2 seems safer.
> >
> 
> I vote for the second version too.

  me too :-) ACK on 2/2

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 1/2] virCommand: document behavior on no output

2010-12-04 Thread Matthias Bolte
2010/12/4 Justin Clift :
> On 04/12/2010, at 8:28 AM, Eric Blake wrote:
>> Option 1: This patch (all callers have to worry about NULL buffers,
>> but checking for output is a simple pointer check).
>>
>> Option 2: Guarantee that outbuf/errbuf are allocated, even if to
>> the empty string.  Caller always has to free the result, and
>> empty output check requires checking if *outbuf=='\0'.
>>
>> Personally, I prefer option 2.  Thoughts?
>
> 2 seems safer.
>

I vote for the second version too.

Matthias

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/2] virCommand: document behavior on no output

2010-12-03 Thread Justin Clift
On 04/12/2010, at 8:28 AM, Eric Blake wrote:
> Option 1: This patch (all callers have to worry about NULL buffers,
> but checking for output is a simple pointer check).
> 
> Option 2: Guarantee that outbuf/errbuf are allocated, even if to
> the empty string.  Caller always has to free the result, and
> empty output check requires checking if *outbuf=='\0'.
> 
> Personally, I prefer option 2.  Thoughts?

2 seems safer.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 1/2] virCommand: document behavior on no output

2010-12-03 Thread Eric Blake
Option 1: This patch (all callers have to worry about NULL buffers,
but checking for output is a simple pointer check).

Option 2: Guarantee that outbuf/errbuf are allocated, even if to
the empty string.  Caller always has to free the result, and
empty output check requires checking if *outbuf=='\0'.

Personally, I prefer option 2.  Thoughts?

* docs/internals/command.html.in: Update documentation.
* src/util/command.c (virCommandSetOutputBuffer)
(virCommandSetErrorBuffer) Guarantee NULL buffer on no output.
---
 docs/internals/command.html.in |4 ++--
 src/util/command.c |2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in
index c4fa800..ea9ec64 100644
--- a/docs/internals/command.html.in
+++ b/docs/internals/command.html.in
@@ -349,8 +349,8 @@

 
   Once the command has finished executing, these buffers
-  will contain the output. It is the callers responsibility
-  to free these buffers.
+  will contain the output, or be NULL if there was no output. It
+  is the callers responsibility to free these buffers.
 

 Setting working directory
diff --git a/src/util/command.c b/src/util/command.c
index aa43f76..1923799 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -549,6 +549,7 @@ virCommandSetOutputBuffer(virCommandPtr cmd, char **outbuf)
 return;
 }

+VIR_FREE(*outbuf);
 cmd->outbuf = outbuf;
 cmd->outfdptr = &cmd->outfd;
 }
@@ -569,6 +570,7 @@ virCommandSetErrorBuffer(virCommandPtr cmd, char **errbuf)
 return;
 }

+VIR_FREE(*errbuf);
 cmd->errbuf = errbuf;
 cmd->errfdptr = &cmd->errfd;
 }
-- 
1.7.3.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list