On Wed, Feb 26, 2014 at 6:10 PM, Max Woodbury mtewoodbury@gmail.com wrote:
... So it's mainly on style:
I've been reading other peoples code for 50 years and that aspect of K&R is much easier to handle in my opinion. Same with the extra blank line. Other parts of wine use the style and are much easier to read as a result.
And the miscount can't simply be ignored?
Very little of Wine uses a blank line after an if, and I've never seen it with a single line statement. That doesn't really matter though, the rest of the function (and really the whole file) is in Allman style and our current guidelines require matching the surrounding style. (Note: I am not in charge of this, so debating me on the topic is pointless)
If you read kernel code, you'll find that just returning to application level is sometimes enough to free the missing resource. That happens often enough that a single quick retry on EAGAIN is warranted.
I'm not familiar enough with what you're trying to do, but if the buffer is full and the kernel throws back EAGAIN then it would seem that you're not in such a time crunch that waiting for the higher level to handle it is going to be a problem. I could be wrong though, I don't do a lot of high-throughput socket programming.
Bringing the relevant part of the retry code down and applying it to only those parts where is has a strong likelihood of doing the most good is exactly what the patch does. If one retry is not enough, then higher level strategy is needed so no changes are called for on the higher level. A loop at this level is _not_ a good idea since there would have to be something to break the loop if it runs too long.
I believe that Ken was suggesting that there are ways to do this such that the higher level can handle EGAIN and the lower level EINTR. In my, admittedly limited, experience with this issue his suggestion seems reasonable, but I could be wrong. However, I am sure that if you put together a convincing test case that it will make getting the patch accepted a lot easier (though not guaranteed).
In other words, this fiddle is based on several decades of experience with kernel and bottom level application debugging experience. At least look at it and test it before rejecting it on superficial grounds.
Please do not be confused, I am not in charge of acceptance/rejection. I have, however, been involved in Wine long enough to know that if you don't address the issues mentioned on this list (either with an explanation, the addition of new tests, or changing the code to handle the case mentioned) then AJ will usually reject the patch.
Best, Erich