Re: [Xen-devel] [PATCH v2] libxl: reimplement buffer for bootloading and drop data if buffer is full

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Roger Pau Monne writes ("[Xen-devel] [PATCH v2] libxl: reimplement buffer for 
bootloading and drop data if buffer is full"):
> +        /* Move buffers around to drop already consumed data */
> +        if (xenconsoled_cons > 0) {
> +            xenconsoled_prod -= xenconsoled_cons;
> +            memmove(xenconsoled_buf, &xenconsoled_buf[xenconsoled_cons], 
> xenconsoled_prod);

This has quite a few overly-long lines.  Can you keep them to 75ish
please ?

> -        ret = select(nfds, &rsel, &wsel, NULL, NULL);
> +        ret = select(nfds, &rsel, &wsel, NULL, &wait);
>          if (ret < 0)
>              goto out_err;

This needs to handle EINTR.  Ie, if EINTR happens, just loop again.
(A better implementation would actually subtract the timeout but
let's not do that now).

> +        timeout = ret == 0 ? 1 : 0;

This seems a slightly odd approach.  How about
  if (ret==0) {
     empty the ring buffer
     continue;
  }
?

And you should avoid setting a timeout if the buffer is empty, so that
a completely idle setup just sits and waits rather than polling every
second.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel