On 24 June 2014 00:06, Paul Burton <p...@archlinuxmips.org> 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
