Am 13.11.2014 um 00:25 hat Eric Blake geschrieben:
On 11/12/2014 01:27 PM, Markus Armbruster wrote:
+ /* in hole, end not yet known */
+ offs = lseek(s->fd, start, SEEK_DATA);
+ if (offs < 0) {
+ /* no idea where the hole ends, give up (unlikely to happen) */
+ goto dunno;
+ }
+ assert(offs >= start);
+ *hole = start;
+ *data = offs;
This assertion feels like an off-by-one. The same offset cannot be both
a hole and data (except in some racy situation where some other process
is writing data to that offset in between our two lseek calls, but
that's already in no-man's land because no one else should be writing
the file while qemu has it open). Is it worth using 'assert(offs >
start)' instead?
As soon as you say "except", it's wrong to assert this at all. We can't
guarantee that the condition is true and it's not a programming error
in qemu if it's false. Sounds to me as if it should be a normal error
check rather than an assertion.
Also, what happens after EOF? I haven't read the patch yet, maybe it
handles the situation already earlier, but if it doesn't, won't we get
offset == start then?