Wine Developer Cheatsheet, v1

[Robert Shearman]

Mike Hearn wrote:

>> The second thing is about the coding style. As I said before, I have 
>> a patch
>> for shlwapi which fixes some MSXML/Borland XML Components problems 
>> with the
>> builtin one, which wasn't accepted for some problem about the style 
>> and the
>> placement of the variables. I posted it as a bug at
>> http://bugs.winehq.org/show_bug.cgi?id=2385 . If you can add something
>> about the coding style I can fix it up and repost it for inclusion.
>
>
> A few things that jump out at me (not really reviewed the code itself):
>
> - No C++ // style comments, I'm afraid. They aren't portable enough.

Yes, as Mike says we don't allow these because they are C99 only and we 
don't like making Wine unnecessarily unportable across compilers.

>
> - Use HeapAlloc() over malloc(), as it says in the cheatsheet
> - Are you sure there is a debugstr_a?? I can't think what it would do,
>   ANSI strings can just be printed by regular printf 

Yes. It is useful for printing large strings that would otherwise 
overflow the static buffer and for strings that contain non-ASCII 
characters.

>
> - You have to declare all the variables at the start of a block. Yes,
>   it's a pain. Another C portability rule. So, can't do this:
>
>   LPSTR foo = bar;
>   StrCpyA(a, b);
>   int i;
>
>   They all have to go at the top.

I also finder easier to read functions that have all the variables 
declared at the top of each block. Don't know the type of a variable? 
Then scan the top of each block - no need to look at any lines in between.

> - Minor stylistic note: you can use strcpy rather than StrCpyA. But I
>   doubt this was a problem.
> - Usually best to space stuff out, ie rather than this:
>
>   if ( temp_out == temp_out2 ) { temp_in += 3; continue; }
>
>   use this:
>
>   if ( temp_out == temp_out2 )
>   {
>       temp_in += 3;
>       continue;
>   }
>
>   ... again I doubt Alexandre would reject the patch because of this.
>
> - //if ( *temp_in == '.' )  ... wine code generally doesn't do this. If
>   you find it hard to match up a closing brace with the statement that
>   opened it maybe you need ... *fanfare* EMACS! :)

Heh, this is fairly similar to Alexandre's style so be careful :)
Although his is more like:
if (cond)
     func( args );

> - Is it possible to implement the ANSI version by converting to unicode
>   then calling the wide version, rather than duplicate it?

Most of the time it is worth it, but probably not in this case and for 
other string processing functions. The bad thing is that the A and W 
functions could get out of sync, so it is best to make sure they work 
perfectly (and don't have any fixme's) before the patch is accepted.

One additional niggle: temp_in and temp_out aren't terribly good 
variable names.

Rob