[libvirt] [PATCH] Eliminate large stack buffer in doTunnelSendAll

[Eric Blake]

On 03/05/2010 02:30 PM, Laine Stump wrote:
> doTunnelSendAll function (used by QEMU migration) uses a 64k buffer on
> the stack, which could be problematic. This patch replaces that with a
> buffer from the heap.

There's a lot more large stacks noted by Coverity, but this is a good start.

> 
> While in the neighborhood, this patch also improves error reporting in
> the case that saferead fails - previously, virStreamAbort() was called
> (resetting errno) before reporting the error. It's been changed to
> report the error first.

Yep.

> -    char buffer[65536];
> -    int nbytes = sizeof(buffer);
> +    char *buffer;
> +    int nbytes = TUNNEL_SEND_BUF_SIZE;
> +
> +    if (VIR_ALLOC_N(buffer, TUNNEL_SEND_BUF_SIZE) < 0) {
> +        virStreamAbort(st);
> +        virReportOOMError();

Is this backwards?

> +        return -1;
> +    }
>  
>      /* XXX should honour the 'resource' parameter here */
>      for (;;) {
>          nbytes = saferead(sock, buffer, nbytes);
>          if (nbytes < 0) {
> -            virStreamAbort(st);
>              virReportSystemError(errno, "%s",
>                                   _("tunnelled migration failed to read from qemu"));
> +            virStreamAbort(st);
> +            VIR_FREE(buffer);

Especially given that you just reordered the virStreamAbort to come last
here?  If the above was not a mistake, then ACK to the patch.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 323 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100305/23b97378/attachment-0001.sig>