Re: PATCH[1/6] user32.dll:SwitchDesktop API Implementation

[Mike Kaplinskiy]

On Sat, Aug 29, 2009 at 1:51 AM, shanmukha sainath
addepalli<sainath.addepalli(a)gmail.com> wrote:
> Hi,
>
>        These are the series of patches for SwitchDesktop and
> OpenInputDesktop API implementation in user32.dll
>
>        SwitchDesktop API when called switches to another desktop and makes
> it active.
>
>        First patch is the implementation of SwitchDesktop API in
> user32/win.c.This function calls the Declaration handler for the
> switchdesktop.
>
> Change Log: Implemented SwitchDesktop API
>
> ---
> Regards
> Sainath A

Sainath,

Some comments (note these are general, I don't actually know the
functions you're implementing):
 - You shouldn't mix tabs and spaces in your patches (match the format
of the file).
 - The wine tree has to compile after every patch (the reason is
regression testing). You can split this queue up into two parts -
first implement OpenInputDesktop, then SwitchDesktop (or the other way
around) - to have 4 patches: 1 - add open_input_desktop server
request, 2 - implement OpenInputDesktop, 3 - add switch_desktop server
request, 4 - implement SwitchDesktop.
 - You don't have to pass the calling thread's tid to the server - the
server has a global called current which is the thread * object that
made the request.
 - You seem to sometimes return error codes from the server, but
changing things nonetheless. Something seems off about that. Also you
sometimes just do return; without setting an error code.
 - I'm not quite sure what you're doing in close_desktop.
 - Code: Don't do if (!(... == ...)), just use if (... != ...). If you
break; in either case of the if, do it outside the if. You don't need
to check &desktop_ops != NULL.
 - Don't use HTML emails, most people don't like those.

Mike.