Official Wine coding style rules/guidelines ?

[Matteo Bruni]

2015-02-13 15:15 GMT+01:00 Indrek Altpere <efbiaiinzinz at hotmail.com>:
> Hello
>

Hi Indrek,
first of all, thanks for the email. I'm not sure there is any
"official" Wine style guideline or that I know what that is. Here's my
view though.

> Would it be possible to get more of the guidelines written down on the
> http://wiki.winehq.org/Developers-Hints or
> http://wiki.winehq.org/SubmittingPatches pages, to get them set in stone,
> so-to-speak? And also somehow show which components have different
> guidelines and what are they?
> Right now, all the information seems to get reiterated to each new
> developer, causing a lots of overhead to the reviewers.

Sure, updating the wiki seems like a great idea. Notice that everyone
can update it so feel free to just go ahead and write down anything
you think it would be useful.

> After being subscribed to this mailing list for a few months, I have noticed
> a pattern with new developers trying to submit patches:
> 1) first patches tend to receive lots of style related feedback
> 2) due to the big list of changes requested, the developers tend to miss a
> few with their resend and have to re-resend the patches again
>          or due to the initial patches being too "out-of-style" the
> reviewers missed a few places in the first round and point them out in later
> round, causing the re-resend
> 3) sometimes the cycle goes on for quite a while
>
> And to confuse things more:
> 1) sometimes different reviewers have different/conflicting opinions about
> style rules
> 2) some rules are component specific, mostly set by the maintainer

Those two points really go together. You're right, different reviewers
(and consequently the components they are responsible of) tend to use
different styles. I agree it might be confusing but it looks like
using a single style for all of wine / reformatting all the
nonconforming code is simply not going to happen so we have to stick
with that. FWIW it's not uncommon for other open source projects to
use different code styles in different components, for one reason or
another.

> 3) http://wiki.winehq.org/SubmittingPatches has only general rules
> 3.1) "Follow the style of surrounding code as much as possible, except when
> it is completely impractical to do so" - there have been many cases where
> patches get rejected because although it was done in the style of
> surrounding code, the component (or wine overall) has adopted new style
> rules, which are not evident in the surrounding code at all.

This is very true. I guess the actual "rule" is more like "follow the
style of recent patches touching surrounding code / in the same DLL".
As an example, there are chunks of wined3d code which are very old and
use different styles. If you are going to send a wined3d patch
touching one of those chunks you generally want to use the current
style anyway. You generally shouldn't reformat code just to make it
conform to the newer style, but you can / should reformat lines you're
touching anyway. It might look funny on the edge between old and new
code, in that case you are usually free to do some adjustments.
You certainly want to use the new style for new functions, regardless
of what the surrounding code looks like.

> 3.2) as a minor issue, the reference to Developer-Hints in the guidelines is
> not linked to http://wiki.winehq.org/Developers-Hints
>
>
> Lots of work has already been done with the Developer-Hints page, but many
> issues still come up in the mailinglist that have not been mentioned there.
>
> UINT vs unsigned int, int* foo vs int *foo, -1 vs ~0U, function parameter
> naming (Hungarian notation ? Ok for API function definitions based on MSDN
> doc or not?).
> Officially LONG use is suggested because long differs between 64bit linux
> and 64bit windows. Should the developers also BOOL FLOAT etc uppercase
> "safe" defines everywhere or not? true/false vs TRUE/FALSE?

AFAIK Hungarian notation is always best avoided. Of course when you
need to preserve API compatibility (e.g. public structure fields)
there is no way around it. I don't think anyone wants to see int* foo
either.
LONG vs long is not a suggestion but a requirement if it's something
affecting the API, because of the 64-bit implementation differences
(i.e. LLP64 vs LP64). About the other data types I guess we're not
entirely consistent in general. We usually prefer to avoid the caps
data types but just try to follow the preexisting conventions in that
DLL and you should be fine.
There are no lowercase true and false in C89 so you need the uppercase
constants. No discussions here ;)

> It also seems that (at least in D3D related code) LP* usages are not
> allowed, LPWSTR => const WCHAR * should be used for parameters.
> But for example in kernel32/volume.c the LPWSTR is used in 10 places as
> function parameter, const WCHAR * is used in only one place as function
> parameter.
> Is the LP* rule overall rule, is it D3D specific? If overall, then why is it
> so widely used in all the kernel files? If D3D specific, how does the
> developer know it beforehand?

LPFOO types are particularly frowned upon since they are obfuscating
the pointers and tend not to do the expected thing with const.
I haven't checked but I guess that they are used so much in kernel32
just because they are in older code.

> Suggestions, comments etc ?
>
> Regards,
> Indrek Altpere

Thanks again for bringing this up. Hopefully if I wrote something
wrong / stupid someone is going to correct me. Also this kind of
discussion is often a recipe for flamewars, so I put my fire-resistant
armor on just in case :P