[PATCH] netio.sys: Remove WINAPI on static functions where not needed.

[Francois Gouget]

Signed-off-by: Francois Gouget 
---
 dlls/netio.sys/netio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/netio.sys/netio.c b/dlls/netio.sys/netio.c
index 53d74f994db..fe73b06b978 100644
--- a/dlls/netio.sys/netio.c
+++ b/dlls/netio.sys/netio.c
@@ -605,7 +605,7 @@ static void WINAPI send_receive_callback(TP_CALLBACK_INSTANCE *instance, void *s
     unlock_socket(socket);
 }

-static NTSTATUS WINAPI do_send_receive(WSK_SOCKET *socket, WSK_BUF *wsk_buf, ULONG flags, IRP *irp, BOOL is_send)
+static NTSTATUS do_send_receive(WSK_SOCKET *socket, WSK_BUF *wsk_buf, ULONG flags, IRP *irp, BOOL is_send)
 {
     struct wsk_socket_internal *s = wsk_socket_internal_from_wsk_socket(socket);
     struct wsk_pending_io *io;

[Paul Gofman]

On 6/29/20 18:51, Francois Gouget wrote:
> Signed-off-by: Francois Gouget
>
> dlls/netio.sys/netio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/dlls/netio.sys/netio.c b/dlls/netio.sys/netio.c
> index 53d74f994db..fe73b06b978 100644
> --- a/dlls/netio.sys/netio.c
> +++ b/dlls/netio.sys/netio.c
> @@ -605,7 +605,7 @@ static void WINAPI send_receive_callback(TP_CALLBACK_INSTANCE *instance, void *s
>      unlock_socket(socket);
>  }
>
> -static NTSTATUS WINAPI do_send_receive(WSK_SOCKET *socket, WSK_BUF *wsk_buf, ULONG flags, IRP *irp, BOOL is_send)
> +static NTSTATUS do_send_receive(WSK_SOCKET *socket, WSK_BUF *wsk_buf, ULONG flags, IRP *irp, BOOL is_send)
>  {
>      struct wsk_socket_internal *s = wsk_socket_internal_from_wsk_socket(socket);
>      struct wsk_pending_io *io;

I don't think it matters much, but is it really an improvement? If we
call a single helper function from WINAPI function, which in turn calls
just WINAPI functions, isn't it cleaner to keep the same calling ABI to
potentially reduce the number of register saves?

[Ken Thomases]

On Jun 29, 2020, at 11:00 AM, Paul Gofman  wrote:
> On 6/29/20 18:51, Francois Gouget wrote:
> > Signed-off-by: Francois Gouget
> >
> > dlls/netio.sys/netio.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/dlls/netio.sys/netio.c b/dlls/netio.sys/netio.c
> > index 53d74f994db..fe73b06b978 100644
> > --- a/dlls/netio.sys/netio.c
> > +++ b/dlls/netio.sys/netio.c
> > @@ -605,7 +605,7 @@ static void WINAPI send_receive_callback(TP_CALLBACK_INSTANCE *instance, void *s
> >     unlock_socket(socket);
> > }
> >
> > -static NTSTATUS WINAPI do_send_receive(WSK_SOCKET *socket, WSK_BUF *wsk_buf, ULONG flags, IRP *irp, BOOL is_send)
> > +static NTSTATUS do_send_receive(WSK_SOCKET *socket, WSK_BUF *wsk_buf, ULONG flags, IRP *irp, BOOL is_send)
> > {
> >     struct wsk_socket_internal *s = wsk_socket_internal_from_wsk_socket(socket);
> >     struct wsk_pending_io *io;
>
> I don't think it matters much, but is it really an improvement? If we
> call a single helper function from WINAPI function, which in turn calls
> just WINAPI functions, isn't it cleaner to keep the same calling ABI to
> potentially reduce the number of register saves?

For a static function, the compiler can choose a non-standard ABI for the function call, to potentially completely avoid register spill.  I don't know if it does or not, but an explicit calling convention could prevent such optimizations.  Best to let the compiler choose.

-Ken

[Francois Gouget]

On Mon, 29 Jun 2020, Paul Gofman wrote:
[...]
> I don't think it matters much, but is it really an improvement? If we
> call a single helper function from WINAPI function, which in turn calls
> just WINAPI functions, isn't it cleaner to keep the same calling ABI to
> potentially reduce the number of register saves?

This check was originally created because gcc had a bug on the Mac which 
caused it to mishandle 'static WINAPI' functions in some cases. Ken 
actually provided the following explanation at the time:

| On the Mac, WINAPI (and other declspecs) include the 
| force_align_arg_pointer attribute. With -O2 optimization (actually 
| -funit-at-a-time, implied by -O2) the compiler may change the calling 
| convention used by non-exported functions to take arguments via 
| registers rather than on the stack. The prologue generated for 
| force_align_arg_pointer clobbers one of those registers (and hence 
| one of the arguments).

That was 12 years ago mind you so presumably gcc has been fixed 
since then. But this does hint that the compiler may indeed adjust 
the calling convention to optimise things.

Also note that static functions that are exported through vtables or 
other pointer passing methods are not impacted because the compiler is 
smart enough to realise it cannot change the calling convention at will.