Re: [Qemu-devel] [PATCH] linux-user: fix ipc(SEMCTL, ...) argument handling

[Peter Maydell]

On 24 June 2014 00:06, Paul Burton <address@hidden> wrote:
> On Mon, Jun 23, 2014 at 11:35:17PM +0100, Peter Maydell wrote:
>> and so I'm dubious about a patch that's
>> trying to make a very small change to it
>
> Isn't that precisely how good bisectable bug fixes should be made?

The key is in the second half of this sentence:

>> without looking
>> at the larger picture and testing and fixing bugs on more
>> than one architecture.
>
> I pointed you to the kernel code which dereferences the pointer & it's
> quite clearly architecture neutral, so I'm not sure what you're getting
> at with the architecture comment.
>
> There's quite clearly a bug in QEMU here, and this patch fixes it. I
> hope you're not saying you don't want it merged because it doesn't fix
> some other hypothetical bugs in the same area.

What I mean is that I would expect that any attempt to fix
behaviour in this area ought to result in a series of three
or four patches which fix various bugs (of which this
would just be one). When an area of code is pretty
clearly bogus like this one, then in my experience an
attempt to make a small fix to it without just going ahead
and overhauling it is likely to result in accidentally
breaking existing working uses which happened to work
more or less by fluke. This is particularly true if you only
test one guest architecture; you can reasonably make that
level of limited testing in areas where the codebase is
sane, but not where it is clearly dubious.

So yes, I would prefer this not to be merged unless either
(a) it comes as part of a series that cleans up the code for
handling semctl so it's not obviously broken
(b) it has been tested to confirm that it doesn't obviously
regress any guest architecture (or at least not any of the
more important ones),
and ideally both.

I don't think this is an enormous amount of work (maybe a
couple of days?); I'm really just recommending the approach
and amount of cleanup that I would do if I found I needed
to make a fix in this area myself.

thanks
-- PMM