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

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.