On 24 June 2014 00:53, Paul Burton <p...@archlinuxmips.org> wrote: > Well I disagree with your logic, but perhaps that's primarily because of > your claim that the semctl code is "clearly bogus" and "obviously > broken". Could you back that up? I know there's the one bogus line in > the GETVAL/SETVAL case that was mentioned in another email, but is there > anything else you consider broken?
The type of the parameter we pass doesn't match what the kernel does, there's been at least one previous attempt to fix stuff in this area, and as you say the getval/setval stuff is broken. That's three things, which means (to my mind) that the first thing that needs to be done is examine the whole routine and determine how it ought to work, independent of what it happens to be doing now. Then you can implement the necessary fixes. I *don't* think this is a big job, or even that the code needs to be completely rewritten. > I see your point on testing, but frankly this code is generic for all > architectures in Linux. I don't have the time to test each architecture > and I don't have the time to test all software that may have previously > been working by fluke. So what's the bar you'd like to set here? Riku's the submaintainer here, so it's his decision in the end (and I think he has a set of tests he runs on patches as well), but one of the bars I have for reviewing patches is that I should be reasonably confident it won't regress existing behaviour for anybody. A simple patch to existing clear and correct code gives me that confidence; a set of patches that take a holistic approach to an obvious trouble spot do too; a pile of testing ditto. A tiny point fix added to something we know to be dubious doesn't give any confidence that it's going to interact correctly with the dubious code. > But anyway, please do enlighten me: where are the bugs of which you > speak? I'd like to see them fixed too :) You're essentially asking me to do the work for you. This is a recipe for me to say "sorry, I don't think we should accept your patch" -- it's you that has a reason to get this patch accepted, not me. thanks -- PMM
