[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date
From: |
Jean-Christophe Dubois |
Subject: |
Re: [Qemu-devel] [PATCH] avoid compilation warning/errors on up to date compilers |
Date: |
Tue, 16 Jun 2009 22:01:35 +0200 |
User-agent: |
KMail/1.11.2 (Linux/2.6.28-13-generic; KDE/4.2.2; x86_64; ; ) |
Le mardi 16 juin 2009 20:23:59 Stuart Brady, vous avez écrit :
> On Tue, Jun 16, 2009 at 06:03:43PM +0300, Riku Voipio wrote:
> > Which tree against did you make these changes? the qemu-ndb.c bit didn't
> > apply against HEAD.
It is whatever you get when cloning the qemu tree with "git clone
git://git.savannah.nongnu.org/qemu.git" as proposed on the savannah web site.
> > Also, there are some whitespace/tab issues. See the
> > CODING_STYLE doc. Functionally I can verify it removes compiler warnings
> > when using a modenr glibc (2.9).
I'll have a look at the CODING_STYLE but I was under the impression I didn't
use the tab stuff. I'll double check.
> Unfortunately, some of the changes don't look right.
>
> For instance:
> > > - read(s->fd, &bitmap_entry, 1);
> > > + if (read(s->fd, &bitmap_entry, 1) != 1)
> > > + return -1; // not allocated
>
> If read() returns -1 and errno is set to EINTR, then the read() should
> be reattempted.
I can agree with this. However you will have to admit that even if this
particular error case is not handled as it should be it is still a big
improvement as there was no error handling at all previously. The main goal
here is to allow that qemu compilation on up to date systems like Ubuntu 9.04
(my case).
> > > - write(cow_fd, &cow_header, sizeof(cow_header));
> > > + if (write(cow_fd, &cow_header, sizeof(cow_header)) ==
> > > sizeof(cow_header)) + goto fail;
>
> Doesn't write(...) == sizeof(cow_header) indicate success?
OOPS, my bad. I'll fix it.
> Also, I gather partial reads/writes need to be supported.
Once again, previously there was no error handling at all. What I propose is
not perfect but at least it does some error handling and allow the qemu
compilation on my system.
Supporting partial read/write would certainly require a lot more code
refactoring.