At 07/13/2011 10:43 AM, Daniel Veillard Write:
> On Tue, Jul 12, 2011 at 02:09:34PM -0600, Eric Blake wrote:
>> virCommandTransferFD promises that the fd is no longer owned by
>> the caller. Normally, we want the fd to remain open until the
>> child runs, but in error situations, we must close it earlier.
>>
>> * src/util/command.c (virCommandTransferFD): Close fd now if we
>> can't track it to close later.
>> ---
>> src/util/command.c | 10 ++++++++--
>> 1 files changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/util/command.c b/src/util/command.c
>> index 3c516ec..83d4e88 100644
>> --- a/src/util/command.c
>> +++ b/src/util/command.c
>> @@ -738,7 +738,7 @@ virCommandKeepFD(virCommandPtr cmd, int fd, bool transfer)
>> void
>> virCommandPreserveFD(virCommandPtr cmd, int fd)
>> {
>> - return virCommandKeepFD(cmd, fd, false);
>> + virCommandKeepFD(cmd, fd, false);
>> }
>
> I must admit I'm surprized, the compiler really doesn't warn if
> one does "return f()" if the caller is a void function. I.e. void
If f() and caller are void functions, gcc does not warn 'return f()'.
If f() is not a void function, and caller is a void function, gcc will
warn 'return f()'
I guess gcc check the caller's return type and f()'s return type. If
they are the same type, gcc does not warn.
> is actually considered a value ??? I guess my brain still follows
> Pascal where procedure and functions were not the same...
>
>> /*
>> @@ -749,7 +749,13 @@ virCommandPreserveFD(virCommandPtr cmd, int fd)
>> void
>> virCommandTransferFD(virCommandPtr cmd, int fd)
>> {
>> - return virCommandKeepFD(cmd, fd, true);
>> + virCommandKeepFD(cmd, fd, true);
>> + if ((!cmd || cmd->has_error) && fd > STDERR_FILENO) {
>> + /* We must close the fd now, instead of waiting for
>> + * virCommandRun, if there was an error that prevented us from
>> + * adding the fd to cmd. */
>> + VIR_FORCE_CLOSE(fd);
>> + }
>> }
>
> Hum, it's a bit like memory allocation, it's better to have the one
> who allocates it to free it to avoid double frees. Maybe we could
> instead raise and error in the caller chain, and have it freed at teh
> right place (unless it really get messy).
>
> opinion ?
>
> Daniel
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list