Svante Signell, le Tue 12 May 2015 10:00:26 +0200, a écrit : > On Tue, 2015-05-12 at 09:42 +0200, Samuel Thibault wrote: > > Svante Signell, le Tue 12 May 2015 09:15:46 +0200, a écrit : > > > The idea is to limit the openmodes according to the values defined in > > > struct netnode and to change the test for overlapping sets. > > > > In which case is this needed? What happens in that case and how the > > patch fixes this? > > I already supplied a test case. What's up?
Explain what happens. Only providing a testcase means the reviewer has to - understand what the testcase is doing - infer how the patched source code gets triggered - find out how that source code does things wrong - understand why the proposed changes fixes that behavior Which takes a lot of time, while the submitter already know all this and just has to explain it. For instance here, just inventing dumb things, it's just to give an example of what I need to see explained to avoid having to spend a few hours to understand what's happening: the testcase creates a socket, whose mode is not just read/write, and thus it test at line 1234 it erroneously takes the "then" branch, while it shouldn't, since it's just a socket. With the proposed changes, the function will just reject the mode, and thus the caller will correctly revert back to code path "bar" which does things properly: it calls foobaz(), which is what we want for the testcase. Really, doing this won't take you much time, and save the reviewer a lot of time (which is just duplicate since it's exactly what you did in your work). Samuel