Re: [ros-dev] [ros-diffs] [akhaldi] 64994: [ADVAPI32] * Update ImpersonateNamedPipeClient(). CORE-8540

2014-10-27 Thread Aleksey Bragin
It was wrong to address this to Amine, because it's not his code 
(neither old, nor new). Well, my question still stands of course, but 
it's up to the original author/maintainter of that code part to decide 
what to do with it :-)


I'm not keen on that change, but if Eric likes it and it makes future 
work easier - pretty cool then.


Regards,
Aleksey

On 26.10.2014 23:07, Aleksey Bragin wrote:
Sometimes, If it ain't broke, don't fix it is a good thing. Or in 
other case, specify what does this sync actually fix.
To me (and I'm known fan of Wine code, just look into Arwinss) it 
plainly looks like Winisation of our own, good code (the style, 
function behavior, whatever else matched ReactOS coding style, 
development practices, etc).


Amine - please fix this any way you prefer. Thanks!

Regards,
Aleksey Bragin

On 26.10.2014 0:10, Ged Murphy wrote:

Eeww, these are a bit ugly :(


On 25/10/2014 19:30, akha...@svn.reactos.org akha...@svn.reactos.org
wrote:


Author: akhaldi
Date: Sat Oct 25 18:30:05 2014
New Revision: 64994

URL: http://svn.reactos.org/svn/reactos?rev=64994view=rev
Log:
[ADVAPI32]
* Update ImpersonateNamedPipeClient().
CORE-8540

Modified:
trunk/reactos/dll/win32/advapi32/wine/security.c

Modified: trunk/reactos/dll/win32/advapi32/wine/security.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/wine/s 


ecurity.c?rev=64994r1=64993r2=64994view=diff
== 



--- trunk/reactos/dll/win32/advapi32/wine/security.c [iso-8859-1]
(original)
+++ trunk/reactos/dll/win32/advapi32/wine/security.c [iso-8859-1] 
Sat Oct

25 18:30:05 2014
@@ -954,37 +954,14 @@
 return TRUE;
}

-/** 


- * ImpersonateNamedPipeClientEXPORTED
- *
- * @implemented
- */
-BOOL
-WINAPI
-ImpersonateNamedPipeClient(HANDLE hNamedPipe)
-{
-IO_STATUS_BLOCK StatusBlock;
-NTSTATUS Status;
-
-TRACE(ImpersonateNamedPipeClient() called\n);
-
-Status = NtFsControlFile(hNamedPipe,
- NULL,
- NULL,
- NULL,
- StatusBlock,
- FSCTL_PIPE_IMPERSONATE,
- NULL,
- 0,
- NULL,
- 0);
-if (!NT_SUCCESS(Status))
-{
-SetLastError(RtlNtStatusToDosError(Status));
-return FALSE;
-}
-
-return TRUE;
+BOOL WINAPI ImpersonateNamedPipeClient( HANDLE hNamedPipe )
+{
+IO_STATUS_BLOCK io_block;
+
+TRACE((%p)\n, hNamedPipe);
+
+return set_ntstatus( NtFsControlFile(hNamedPipe, NULL, NULL, NULL,
+ io_block, FSCTL_PIPE_IMPERSONATE, NULL, 0,
NULL, 0) );
}

/*









___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [akhaldi] 64994: [ADVAPI32] * Update ImpersonateNamedPipeClient(). CORE-8540

2014-10-27 Thread Aleksey Bragin

To be fair, it doesn't:

static __inline BOOL set_ntstatus( NTSTATUS status )
{
if (!NT_SUCCESS(status)) SetLastError( RtlNtStatusToDosError( status ));
return NT_SUCCESS(status);
}

So essentially it's just refactoring.

Regards,
Aleksey

On 26.10.2014 0:25, Pierre Schweitzer wrote:

And it changes behavior of the functions.

I'd rather trust a !NT_SUCESS(Status)) than a status. This means these
functions with Wine code would fail on an informational status (whereas
they didn't before). Is it intentional? Do we have tests to assess such
behavior?

It's a bit a shame to replace well written ReactOS to Wine code with
broken style :-(.

On 25/10/2014 22:10, Ged Murphy wrote:

Eeww, these are a bit ugly :(


On 25/10/2014 19:30, akha...@svn.reactos.org akha...@svn.reactos.org
wrote:


Author: akhaldi
Date: Sat Oct 25 18:30:05 2014
New Revision: 64994

URL: http://svn.reactos.org/svn/reactos?rev=64994view=rev
Log:
[ADVAPI32]
* Update ImpersonateNamedPipeClient().
CORE-8540

Modified:
trunk/reactos/dll/win32/advapi32/wine/security.c

Modified: trunk/reactos/dll/win32/advapi32/wine/security.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/wine/s
ecurity.c?rev=64994r1=64993r2=64994view=diff
==

--- trunk/reactos/dll/win32/advapi32/wine/security.c[iso-8859-1]
(original)
+++ trunk/reactos/dll/win32/advapi32/wine/security.c[iso-8859-1] Sat Oct
25 18:30:05 2014
@@ -954,37 +954,14 @@
 return TRUE;
}

-/**
- * ImpersonateNamedPipeClient  EXPORTED
- *
- * @implemented
- */
-BOOL
-WINAPI
-ImpersonateNamedPipeClient(HANDLE hNamedPipe)
-{
-IO_STATUS_BLOCK StatusBlock;
-NTSTATUS Status;
-
-TRACE(ImpersonateNamedPipeClient() called\n);
-
-Status = NtFsControlFile(hNamedPipe,
- NULL,
- NULL,
- NULL,
- StatusBlock,
- FSCTL_PIPE_IMPERSONATE,
- NULL,
- 0,
- NULL,
- 0);
-if (!NT_SUCCESS(Status))
-{
-SetLastError(RtlNtStatusToDosError(Status));
-return FALSE;
-}
-
-return TRUE;
+BOOL WINAPI ImpersonateNamedPipeClient( HANDLE hNamedPipe )
+{
+IO_STATUS_BLOCK io_block;
+
+TRACE((%p)\n, hNamedPipe);
+
+return set_ntstatus( NtFsControlFile(hNamedPipe, NULL, NULL, NULL,
+ io_block, FSCTL_PIPE_IMPERSONATE, NULL, 0,
NULL, 0) );
}

/*







___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [akhaldi] 64994: [ADVAPI32] * Update ImpersonateNamedPipeClient(). CORE-8540

2014-10-26 Thread Aleksey Bragin
Sometimes, If it ain't broke, don't fix it is a good thing. Or in 
other case, specify what does this sync actually fix.
To me (and I'm known fan of Wine code, just look into Arwinss) it 
plainly looks like Winisation of our own, good code (the style, function 
behavior, whatever else matched ReactOS coding style, development 
practices, etc).


Amine - please fix this any way you prefer. Thanks!

Regards,
Aleksey Bragin

On 26.10.2014 0:10, Ged Murphy wrote:

Eeww, these are a bit ugly :(


On 25/10/2014 19:30, akha...@svn.reactos.org akha...@svn.reactos.org
wrote:


Author: akhaldi
Date: Sat Oct 25 18:30:05 2014
New Revision: 64994

URL: http://svn.reactos.org/svn/reactos?rev=64994view=rev
Log:
[ADVAPI32]
* Update ImpersonateNamedPipeClient().
CORE-8540

Modified:
trunk/reactos/dll/win32/advapi32/wine/security.c

Modified: trunk/reactos/dll/win32/advapi32/wine/security.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/wine/s
ecurity.c?rev=64994r1=64993r2=64994view=diff
==

--- trunk/reactos/dll/win32/advapi32/wine/security.c[iso-8859-1]
(original)
+++ trunk/reactos/dll/win32/advapi32/wine/security.c[iso-8859-1] Sat Oct
25 18:30:05 2014
@@ -954,37 +954,14 @@
 return TRUE;
}

-/**
- * ImpersonateNamedPipeClient  EXPORTED
- *
- * @implemented
- */
-BOOL
-WINAPI
-ImpersonateNamedPipeClient(HANDLE hNamedPipe)
-{
-IO_STATUS_BLOCK StatusBlock;
-NTSTATUS Status;
-
-TRACE(ImpersonateNamedPipeClient() called\n);
-
-Status = NtFsControlFile(hNamedPipe,
- NULL,
- NULL,
- NULL,
- StatusBlock,
- FSCTL_PIPE_IMPERSONATE,
- NULL,
- 0,
- NULL,
- 0);
-if (!NT_SUCCESS(Status))
-{
-SetLastError(RtlNtStatusToDosError(Status));
-return FALSE;
-}
-
-return TRUE;
+BOOL WINAPI ImpersonateNamedPipeClient( HANDLE hNamedPipe )
+{
+IO_STATUS_BLOCK io_block;
+
+TRACE((%p)\n, hNamedPipe);
+
+return set_ntstatus( NtFsControlFile(hNamedPipe, NULL, NULL, NULL,
+ io_block, FSCTL_PIPE_IMPERSONATE, NULL, 0,
NULL, 0) );
}

/*







___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [akhaldi] 64994: [ADVAPI32] * Update ImpersonateNamedPipeClient(). CORE-8540

2014-10-25 Thread Ged Murphy
Eeww, these are a bit ugly :(


On 25/10/2014 19:30, akha...@svn.reactos.org akha...@svn.reactos.org
wrote:

Author: akhaldi
Date: Sat Oct 25 18:30:05 2014
New Revision: 64994

URL: http://svn.reactos.org/svn/reactos?rev=64994view=rev
Log:
[ADVAPI32]
* Update ImpersonateNamedPipeClient().
CORE-8540

Modified:
trunk/reactos/dll/win32/advapi32/wine/security.c

Modified: trunk/reactos/dll/win32/advapi32/wine/security.c
URL: 
http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/wine/s
ecurity.c?rev=64994r1=64993r2=64994view=diff
==

--- trunk/reactos/dll/win32/advapi32/wine/security.c   [iso-8859-1]
(original)
+++ trunk/reactos/dll/win32/advapi32/wine/security.c   [iso-8859-1] Sat Oct
25 18:30:05 2014
@@ -954,37 +954,14 @@
 return TRUE;
 }
 
-/**
- * ImpersonateNamedPipeClient EXPORTED
- *
- * @implemented
- */
-BOOL
-WINAPI
-ImpersonateNamedPipeClient(HANDLE hNamedPipe)
-{
-IO_STATUS_BLOCK StatusBlock;
-NTSTATUS Status;
-
-TRACE(ImpersonateNamedPipeClient() called\n);
-
-Status = NtFsControlFile(hNamedPipe,
- NULL,
- NULL,
- NULL,
- StatusBlock,
- FSCTL_PIPE_IMPERSONATE,
- NULL,
- 0,
- NULL,
- 0);
-if (!NT_SUCCESS(Status))
-{
-SetLastError(RtlNtStatusToDosError(Status));
-return FALSE;
-}
-
-return TRUE;
+BOOL WINAPI ImpersonateNamedPipeClient( HANDLE hNamedPipe )
+{
+IO_STATUS_BLOCK io_block;
+
+TRACE((%p)\n, hNamedPipe);
+
+return set_ntstatus( NtFsControlFile(hNamedPipe, NULL, NULL, NULL,
+ io_block, FSCTL_PIPE_IMPERSONATE, NULL, 0,
NULL, 0) );
 }
 
 /*





___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev


Re: [ros-dev] [ros-diffs] [akhaldi] 64994: [ADVAPI32] * Update ImpersonateNamedPipeClient(). CORE-8540

2014-10-25 Thread Pierre Schweitzer
And it changes behavior of the functions.

I'd rather trust a !NT_SUCESS(Status)) than a status. This means these
functions with Wine code would fail on an informational status (whereas
they didn't before). Is it intentional? Do we have tests to assess such
behavior?

It's a bit a shame to replace well written ReactOS to Wine code with
broken style :-(.

On 25/10/2014 22:10, Ged Murphy wrote:
 Eeww, these are a bit ugly :(
 
 
 On 25/10/2014 19:30, akha...@svn.reactos.org akha...@svn.reactos.org
 wrote:
 
 Author: akhaldi
 Date: Sat Oct 25 18:30:05 2014
 New Revision: 64994

 URL: http://svn.reactos.org/svn/reactos?rev=64994view=rev
 Log:
 [ADVAPI32]
 * Update ImpersonateNamedPipeClient().
 CORE-8540

 Modified:
trunk/reactos/dll/win32/advapi32/wine/security.c

 Modified: trunk/reactos/dll/win32/advapi32/wine/security.c
 URL: 
 http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/advapi32/wine/s
 ecurity.c?rev=64994r1=64993r2=64994view=diff
 ==
 
 --- trunk/reactos/dll/win32/advapi32/wine/security.c [iso-8859-1]
 (original)
 +++ trunk/reactos/dll/win32/advapi32/wine/security.c [iso-8859-1] Sat Oct
 25 18:30:05 2014
 @@ -954,37 +954,14 @@
 return TRUE;
 }

 -/**
 - * ImpersonateNamedPipeClient   EXPORTED
 - *
 - * @implemented
 - */
 -BOOL
 -WINAPI
 -ImpersonateNamedPipeClient(HANDLE hNamedPipe)
 -{
 -IO_STATUS_BLOCK StatusBlock;
 -NTSTATUS Status;
 -
 -TRACE(ImpersonateNamedPipeClient() called\n);
 -
 -Status = NtFsControlFile(hNamedPipe,
 - NULL,
 - NULL,
 - NULL,
 - StatusBlock,
 - FSCTL_PIPE_IMPERSONATE,
 - NULL,
 - 0,
 - NULL,
 - 0);
 -if (!NT_SUCCESS(Status))
 -{
 -SetLastError(RtlNtStatusToDosError(Status));
 -return FALSE;
 -}
 -
 -return TRUE;
 +BOOL WINAPI ImpersonateNamedPipeClient( HANDLE hNamedPipe )
 +{
 +IO_STATUS_BLOCK io_block;
 +
 +TRACE((%p)\n, hNamedPipe);
 +
 +return set_ntstatus( NtFsControlFile(hNamedPipe, NULL, NULL, NULL,
 + io_block, FSCTL_PIPE_IMPERSONATE, NULL, 0,
 NULL, 0) );
 }

 /*


 
 
 
 ___
 Ros-dev mailing list
 Ros-dev@reactos.org
 http://www.reactos.org/mailman/listinfo/ros-dev
 


-- 
Pierre Schweitzer pierre at reactos.org
System  Network Administrator
Senior Kernel Developer
ReactOS Deutschland e.V.



smime.p7s
Description: S/MIME Cryptographic Signature
___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev