ws2_32: Add support for FROM_PROTOCOL_INFO to WSASocket()

Kai Blin kai.blin at gmail.com
Mon May 26 02:09:11 CDT 2008


On Monday 26 May 2008 05:08:46 Dmitry Timoshkov wrote:
> "Kai Blin" <kai.blin at 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

-- 
Kai Blin
WorldForge developer  http://www.worldforge.org/
Wine developer        http://wiki.winehq.org/KaiBlin
Samba team member     http://www.samba.org/samba/team/
--
Will code for cotton.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part.
Url : http://www.winehq.org/pipermail/wine-devel/attachments/20080526/392fd4be/attachment.pgp 


More information about the wine-devel mailing list