On Wed, 2011-10-19 at 08:42 +0100, Roger Pau Monné wrote:
> >> if (xenconsoled_prod == xenconsoled_cons)
> >> xenconsoled_prod = xenconsoled_cons = 0;
> >> if (bootloader_prod == bootloader_cons)
> >> bootloader_prod = bootloader_cons = 0;
>
> Now that I also look at it, I think this "ifs" could be removed, the
> following checks for cons > 0 will perform the same task if cons ==
> prod, and the code will look cleaner.
I think you are correct.
> >> /* Input from xenconsole, read xenconsoled_fd, write
> >> bootloader_fd */
> >> - if (FD_ISSET(xenconsoled_fd, &rsel)) {
> >> + if (ret == 0 && xenconsoled_prod == XENCONSOLED_BUF_SIZE) {
> >> + /* Drop the buffer */
> >> + xenconsoled_prod = 0;
> >
> > Do you also need to reset cons here? I expect we could prove it was
> > always zero anyway but it might be more obvious to just reset it.
>
> It is always 0 because we check "cons > 0" at the start of the loop,
> and set it to 0 if it is bigger. If you feel it's best to set it to
> zero here to make the code easier to understand it's fine for me.
So I guess it is more "documentation" than useful code. I think zeroing
cons would be harmless and make the code more obvious.
> > Otherwise this looks good, thanks.
>
> Thanks to you and Ian Jackson for reviewing the code and having so
> much patience.
No problem.
Ian.
>
> >> + } else if (FD_ISSET(xenconsoled_fd, &rsel)) {
> >> ret = read(xenconsoled_fd,
> >> &xenconsoled_buf[xenconsoled_prod], XENCONSOLED_BUF_SIZE -
> >> xenconsoled_prod);
> >> if (ret < 0 && errno != EIO && errno != EAGAIN)
> >> goto out_err;
> >> @@ -229,7 +258,10 @@ static char * bootloader_interact(libxl_
> >> }
> >>
> >> /* Input from bootloader, read bootloader_fd, write
> >> xenconsoled_fd */
> >> - if (FD_ISSET(bootloader_fd, &rsel)) {
> >> + if (ret == 0 && bootloader_prod == BOOTLOADER_BUF_SIZE) {
> >> + /* Drop the buffer */
> >> + bootloader_prod = 0;
> >> + } else if (FD_ISSET(bootloader_fd, &rsel)) {
> >> ret = read(bootloader_fd, &bootloader_buf[bootloader_prod],
> >> BOOTLOADER_BUF_SIZE - bootloader_prod);
> >> if (ret < 0 && errno != EIO && errno != EAGAIN)
> >> goto out_err;
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@xxxxxxxxxxxxxxxxxxx
> >> http://lists.xensource.com/xen-devel
> >
> >
> >
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home