[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH REBASED] Remove main loop



On Mon, Sep 14, 2009 at 06:50:14PM +0100, Matthew Booth wrote:
> On 14/09/09 13:56, Matthew Booth wrote:
>> On 14/09/09 12:10, Richard W.M. Jones wrote:
>>> This is the only patch I currently have outstanding. No changes from
>>> the previous posting, except I rebased it against the head of git.
>>
>> I'm running out of time, so I'm going to dump what I've got:
>>
>> * I already moaned about gotos.
>>
>> * Why does the patch change the set_trace test in generator.ml?
>>
>> * Don't just comment out xread: remove it. These pile up over time and
>> make the code unreadable.
>>
>> * Don't put a usleep() in read_log_message_or_eog. I understand your
>> argument, but in this case the cure is worse than the disease. If you
>> think about it, the patch back into the main loop and back here is
>> actually very short, and very cheap. As well as being ugly as sin, this
>> usleep is only going to slow it down.
>>
>> * guestfs__send and send_file_chunk both leak msg_out in the non-error
>> case.
>>
>> I'll continue until I have to go.
>
> As discussed on IRC:
>
> In recv_from_daemon, why not allocate buf_rtn fully once its size is known?
>
> recv_file_data
>     leak buf if return == -1 (e.g. second read in recv_from_daemon fails)
>
> receive_file_data
>     leak buf if return == -1
>
> Apart from that, I think this patch is the right direction, it clearly  
> works and should get as much testing as possible.
>
> ACK.

Thanks - I fixed the above issues, and ran another round of testing
just to make sure, and after that it still passed, so I will now push
it.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]