On 02/26/2014 08:32 PM, Erich E. Hoover wrote:
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)
Bluntly Allman style sucks. It is just plain hard to read. Also, the blank line is associated with the 'return' and is often used.
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.
Not a buffers full problem. Internal locks that get released from a thread switch and things like that. Worth ONE retry to see if they clear. If its still stuck on the second attempt, punt. And it's not me, it's the game...
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).
TEST CASE?!? Not! Multi-thread with graphics throwing interrupts left and right! Disk interrupts. Real internet delays. Instead, just watch the game with the problem for a day or two. Better read a few good Unix programming text. Ask the impossible why dont'cha.
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.
Yep. If it were possible, I'd do just that, but it's not... On the other hand, forgetting a CC is bad. (Fixed. Want to compare years on the list? I've a few...)
Best, Erich