On Monday 26 May 2008 05:08:46 Dmitry Timoshkov wrote:
"Kai Blin" kai.blin@gmail.com wrote:
- {FROM_PROTOCOL_INFO, FROM_PROTOCOL_INFO},
};
Although it was the case before, it's a mistake to have a comma after the last struct member initializer.
It is? K&R seems to use both, so I preferred to keep it similar to all the other struct member initializers.
I said "a mistake" not "an error". Although the compiler forgives this, what the last comma is serving for?
The C grammar printed in my edition of K&R says the following about initializers
initializer: assignment-expression { initializer-list } { initializer-list , }
so it looks like it's in the C standard that both are perfectly valid. I used the perfectly valid form the rest of the file uses.
- if (lpProtocolInfo) {
if (af == FROM_PROTOCOL_INFO)
af = lpProtocolInfo->iAddressFamily;
if (type == FROM_PROTOCOL_INFO)
type = lpProtocolInfo->iSocketType;
if (protocol == FROM_PROTOCOL_INFO)
protocol = lpProtocolInfo->iProtocol;
Please keep opening brace style/indentation consistent with code you have removed.
The function is a mess with brace style and indentation. I've used the same indentaion as the if right above this one. The function wildly mixes two-space, three-space and four-space indents. In this case, I used two-space indents for the outer if and four-space indents for the inner ifs for better readability.
Even if the overall style is a mess it's not a solution to add the code which mixes 2 and 4 spaces indentation, and the code you have removed used a consistent style.
Ok. Fine. How about we finally agree on a coding standard, document that somewhere and require all new patches to conform to that? Then at least one wouldn't have to guess which broken indentation/braces to follow.
PSDK has the AF_*, PF_*, SOCK_* and IPPROTO_* defines in winsock2.h as well, Wine hasn't. I preferred to keep them together.
How much efforts would it take to put 1 line to a proper .h file?
You're not even looking at what I wrote, are you? FROM_PROTOCOL_INFO is used like an address family define, and I tried to keep all of them together.
All the patches sent to wine-patches fix a bug, not of them are applied due to different reasons. You are not a first timer, and know the rules.
What rules are you referring to? The rule that says when adding code to a file that has no obvious dominating code style, I should pick the code style from below the code I add not the code style from above the code I add? I didn't know that one.
I welcome code review that points out errors in my code. Code review that alternates between requiring me to be "formally correct" and "consistent to the code" in a seemingly random fashion is different.
So you don't care about this patch because you don't have a customer who needs it, I don't care about this patch because I can spend my free time on other things. Let's just drop it then.
Kai