Re: [ros-dev] [ros-diffs] [dchapyshev] 37396: - Implement BindIoCompletionCallback, ReadFileScatter, WriteFileGather (based on Wine) - Move DnsHostnameToComputerNameA/W to computername.c

[gedmurphy]

Steven has repeatedly asked for credit, and I quote:

" Please add the appropriate copyright headers to the source file
stating the original author when implementing code based on third
party sources."

Is there a reason for not doing it this time?

Secondly, why are things being implemented in stubs?

Thirdly, it's worth checking implementations before moving out of stubs. Just because Wine does something, doesn't mean it's correct for us.
DnsHostnameToComputerName should call RtlDnsHostNameToComputerName.

I stopped checking the rest of the code as soon as I read that, in case someone assumes I looked at everything.

Fourthly, I'm willing to bet this email gets ignored too.

Please take a little more time over commits.

Ged.

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of [e-mail]
Sent: 17 November 2008 11:54
To: [e-mail]
Subject: [ros-diffs] [dchapyshev] 37396: - Implement BindIoCompletionCallback, ReadFileScatter, WriteFileGather (based on Wine) - Move DnsHostnameToComputerNameA/W to computername.c

Author: dchapyshev
Date: Mon Nov 17 05:53:59 2008
New Revision: 37396

URL: http://svn.reactos.org/svn/reactos?rev=37396&view=rev
Log:
- Implement BindIoCompletionCallback, ReadFileScatter, WriteFileGather (based on Wine)
- Move DnsHostnameToComputerNameA/W to computername.c

Modified:
    trunk/reactos/dll/win32/kernel32/misc/computername.c
    trunk/reactos/dll/win32/kernel32/misc/stubs.c

Modified: trunk/reactos/dll/win32/kernel32/misc/computername.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/misc/com...
==============================================================================
--- trunk/reactos/dll/win32/kernel32/misc/computername.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/misc/computername.c [iso-8859-1] Mon Nov 17 05:53:59 2008
@@ -461,4 +461,73 @@
   }
 }

+
+/*
+ * @implemented
+ */
+BOOL
+STDCALL
+DnsHostnameToComputerNameA(LPCSTR Hostname,
+                           LPSTR ComputerName,
+                           LPDWORD nSize)
+{
+    DWORD len;
+
+    DPRINT("(%s, %p, %p)\n", Hostname, ComputerName, nSize);
+
+    if (!Hostname || !nSize)
+        return FALSE;
+
+    len = lstrlenA(Hostname);
+
+    if (len > MAX_COMPUTERNAME_LENGTH)
+        len = MAX_COMPUTERNAME_LENGTH;
+
+    if (*nSize < len)
+    {
+        *nSize = len;
+        return FALSE;
+    }
+
+    if (!ComputerName) return FALSE;
+
+    memcpy( ComputerName, Hostname, len );
+    ComputerName[len + 1] = 0;
+    return TRUE;
+}
+
+
+/*
+ * @implemented
+ */
+BOOL
+STDCALL
+DnsHostnameToComputerNameW (
+	LPCWSTR hostname,
+    LPWSTR computername,
+	LPDWORD size
+    )
+{
+    DWORD len;
+
+    DPRINT("(%s, %p, %p): stub\n", hostname, computername, size);
+
+    if (!hostname || !size) return FALSE;
+    len = lstrlenW(hostname);
+
+    if (len > MAX_COMPUTERNAME_LENGTH)
+        len = MAX_COMPUTERNAME_LENGTH;
+
+    if (*size < len)
+    {
+        *size = len;
+        return FALSE;
+    }
+    if (!computername) return FALSE;
+
+    memcpy( computername, hostname, len * sizeof(WCHAR) );
+    computername[len + 1] = 0;
+    return TRUE;
+}
+
 /* EOF */

Modified: trunk/reactos/dll/win32/kernel32/misc/stubs.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/kernel32/misc/stu...
==============================================================================
--- trunk/reactos/dll/win32/kernel32/misc/stubs.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/kernel32/misc/stubs.c [iso-8859-1] Mon Nov 17 05:53:59 2008
@@ -342,18 +342,29 @@
 }

/*
- * @unimplemented
- */
-BOOL
-STDCALL
-BindIoCompletionCallback (
-    HANDLE FileHandle,
-    LPOVERLAPPED_COMPLETION_ROUTINE Function,
-    ULONG Flags
-    )
-{
-    STUB;
-    return 0;
+ * @implemented
+ */
+BOOL
+STDCALL
+BindIoCompletionCallback(HANDLE FileHandle,
+                         LPOVERLAPPED_COMPLETION_ROUTINE Function,
+                         ULONG Flags)
+{
+    NTSTATUS Status = 0;
+
+    DPRINT("(%p, %p, %d)\n", FileHandle, Function, Flags);
+
+    Status = RtlSetIoCompletionCallback(FileHandle,
+                                        (PRTL_OVERLAPPED_COMPLETION_ROUTINE) Function,
+                                        Flags);
+
+    if (!NT_SUCCESS(Status))
+    {
+        SetLastError(RtlNtStatusToDosError(Status));
+        return FALSE;
+    }
+
+    return TRUE;
 }

/*
@@ -562,20 +573,45 @@
 }

/*
- * @unimplemented
- */
-BOOL
-STDCALL
-ReadFileScatter(
-    HANDLE hFile,
-    FILE_SEGMENT_ELEMENT aSegmentArray[],
-    DWORD nNumberOfBytesToRead,
-    LPDWORD lpReserved,
-    LPOVERLAPPED lpOverlapped
-    )
-{
-    STUB;
-    return 0;
+ * @implemented
+ */
+BOOL
+STDCALL
+ReadFileScatter(HANDLE hFile,
+                FILE_SEGMENT_ELEMENT aSegmentArray[],
+                DWORD nNumberOfBytesToRead,
+                LPDWORD lpReserved,
+                LPOVERLAPPED lpOverlapped)
+{
+    PIO_STATUS_BLOCK pIOStatus;
+    LARGE_INTEGER Offset;
+    NTSTATUS Status;
+
+    DPRINT("(%p %p %u %p)\n", hFile, aSegmentArray, nNumberOfBytesToRead, lpOverlapped);
+
+    Offset.LowPart  = lpOverlapped->Offset;
+    Offset.HighPart = lpOverlapped->OffsetHigh;
+    pIOStatus = (PIO_STATUS_BLOCK) lpOverlapped;
+    pIOStatus->Status = STATUS_PENDING;
+    pIOStatus->Information = 0;
+
+    Status = NtReadFileScatter(hFile,
+                               NULL,
+                               NULL,
+                               NULL,
+                               pIOStatus,
+                               aSegmentArray,
+                               nNumberOfBytesToRead,
+                               &Offset,
+                               NULL);
+
+    if (!NT_SUCCESS(Status))
+    {
+        SetLastError(RtlNtStatusToDosError(Status));
+        return FALSE;
+    }
+
+    return TRUE;
 }

/*
@@ -678,20 +714,45 @@
 }

/*
- * @unimplemented
- */
-BOOL
-STDCALL
-WriteFileGather(
-    HANDLE hFile,
-    FILE_SEGMENT_ELEMENT aSegmentArray[],
-    DWORD nNumberOfBytesToWrite,
-    LPDWORD lpReserved,
-    LPOVERLAPPED lpOverlapped
-    )
-{
-    STUB;
-    return 0;
+ * @implemented
+ */
+BOOL
+STDCALL
+WriteFileGather(HANDLE hFile,
+                FILE_SEGMENT_ELEMENT aSegmentArray[],
+                DWORD nNumberOfBytesToWrite,
+                LPDWORD lpReserved,
+                LPOVERLAPPED lpOverlapped)
+{
+    PIO_STATUS_BLOCK IOStatus;
+    LARGE_INTEGER Offset;
+    NTSTATUS Status;
+
+    DPRINT("%p %p %u %p\n", hFile, aSegmentArray, nNumberOfBytesToWrite, lpOverlapped);
+
+    Offset.LowPart = lpOverlapped->Offset;
+    Offset.HighPart = lpOverlapped->OffsetHigh;
+    IOStatus = (PIO_STATUS_BLOCK) lpOverlapped;
+    IOStatus->Status = STATUS_PENDING;
+    IOStatus->Information = 0;
+
+    Status = NtWriteFileGather(hFile,
+                               NULL,
+                               NULL,
+                               NULL,
+                               IOStatus,
+                               aSegmentArray,
+                               nNumberOfBytesToWrite,
+                               &Offset,
+                               NULL);
+
+    if (!NT_SUCCESS(Status))
+    {
+        SetLastError(RtlNtStatusToDosError(Status));
+        return FALSE;
+    }
+
+    return TRUE;
 }

/*
@@ -705,39 +766,6 @@
 {
     STUB;
     return 0;
-}
-
-/*
- * @unimplemented
- */
-BOOL
-STDCALL
-DnsHostnameToComputerNameW (
-	LPCWSTR hostname,
-    LPWSTR computername,
-	LPDWORD size
-    )
-{
-    DWORD len;
-
-    DPRINT("(%s, %p, %p): stub\n", hostname, computername, size);
-
-    if (!hostname || !size) return FALSE;
-    len = lstrlenW(hostname);
-
-    if (len > MAX_COMPUTERNAME_LENGTH)
-        len = MAX_COMPUTERNAME_LENGTH;
-
-    if (*size < len)
-    {
-        *size = len;
-        return FALSE;
-    }
-    if (!computername) return FALSE;
-
-    memcpy( computername, hostname, len * sizeof(WCHAR) );
-    computername[len + 1] = 0;
-    return TRUE;
 }

/*
@@ -919,21 +947,6 @@
 /*
  * @unimplemented
  */
-BOOL
-STDCALL
-DnsHostnameToComputerNameA (
-    LPCSTR Hostname,
-    LPSTR ComputerName,
-    LPDWORD nSize
-    )
-{
-    STUB;
-    return 0;
-}
-
-/*
- * @unimplemented
- */
 HANDLE
 STDCALL
 FindFirstVolumeMountPointA(
@@ -947,23 +960,22 @@
 }

/*
- * @unimplemented
- */
-BOOL
-STDCALL
-FindNextVolumeA(
-	HANDLE handle,
-	LPSTR volume,
-	DWORD len
-    )
-{
-    WCHAR *buffer = HeapAlloc( GetProcessHeap(), 0, len * sizeof(WCHAR) );
+ * @implemented
+ */
+BOOL
+STDCALL
+FindNextVolumeA(HANDLE handle,
+                LPSTR volume,
+                DWORD len)
+{
+    WCHAR *buffer = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
     BOOL ret;

if ((ret = FindNextVolumeW( handle, buffer, len )))
     {
         if (!WideCharToMultiByte( CP_ACP, 0, buffer, -1, volume, len, NULL, NULL )) ret = FALSE;
     }
+
     HeapFree( GetProcessHeap(), 0, buffer );
     return ret;
 }

[Aleksey Bragin]

On Nov 17, 2008, at 4:20 PM, gedmurphy wrote:
> Steven has repeatedly asked for credit, and I quote:
>
> " Please add the appropriate copyright headers to the source file
> stating the original author when implementing code based on third
> party sources."

There is one concern about it: how to find a proper author for a  
specific function he is copying? We can look at the top of the file,  
but there are a number of people listed there, possibly responsible  
for different parts of the file. Should he review all Wine commits  
and pick an author of the patch? What if the specific function was  
patched later by a few authors?
> Is there a reason for not doing it this time?
>
> Secondly, why are things being implemented in stubs?

There is same stuff in gdi32 stub.c having a few implemented  
functions, iirc. Not very good decision, but as long as people  
responsible remember about this.. and keep moving it to appropriate  
places, I don't mind.

WBR,
Aleksey.

[gedmurphy]

Aleksey Bragin wrote
> There is one concern about it: how to find a proper author for a  
> specific function he is copying? We can look at the top of the file,  
> but there are a number of people listed there, possibly responsible  
> for different parts of the file. Should he review all Wine commits  
> and pick an author of the patch? What if the specific function was  
> patched later by a few authors?

This is a good point.
Manually looking through the history is not practical, and too open to
error.

Steven, do Wine have any thoughts on this? 
I only brought this up so it doesn't look like we're ignoring Wine's
requests.

Ged.

[Steven Edwards]

On Mon, Nov 17, 2008 at 8:47 AM, gedmurphy [e-mail] wrote:
> Steven, do Wine have any thoughts on this?
> I only brought this up so it doesn't look like we're ignoring Wine's
> requests.

I propose we try to adopt a simple set of guidelines for how to handle
Wine (or for that matter any other) code that is derived in ReactOS
source files. The KEY thing is we want to keep it fun and not add a
bunch of work otherwise no one will want to do it. If we need to do an
audit later having some simple rules in place will save us some
trouble. I think the rule of thumb should be something like

- if you copy an entire source file from Wine with only minor
changes, such as we've done in a lot of places in advapi32 or
kernel32, leave the original license header with authors place. Add a
GPL header on top for your changes if you hate all things LGPL or add
yourself to the original Wine header with a comment that you did the
ReactOS changes.

- If you copy a function from Wine verbatim or with very minor
changes but are adding that to a source file containing mostly ReactOS
original code, put a comment in the comment block above the function
that its came from Wine (in case we want to audit later). In the
ReactOS license header, attribute the Wine author that you 'think' is
the original author based upon the Wine license header. It's not your
fault if they did not make it clear in the source file which author
implemented what functionality and if you got the Wine source via
tarball or something, its not like you have a method of checking the
revision history anyway.

- If your doing major hacking on a function that is derived from Wine
sources, I don't think its too much trouble for you to fire up a
browser and hit http://source.winehq.org/git/wine.git/?a=tree to see
who the original author is and note this in the license header. Again,
this brings in to question, how much of the original work remains
after your changes? It's a judgement call.  I think the rule of thumb
should be, if you were going to add a comment to SVN saying 'from
Wine' then you need to credit the original author.

- If your work really has no resemblance to the original Wine code,
the original Wine code was totally wrong and you had to rewrite the
whole thing, then you don't need to credit Wine at all. If you just
want to credit them you can always say something like 'inspired by
Wine'

Alexandre tends to avoid accepting formatting changes so using 'git
blame file.c' is pretty safe. Another thing you can try doing is
grepping the Changelog for the function name and see who's credited as
the author. I don't think either of these processes take very long but
I don't think that ReactOS developers need to go through all of the
trouble in every case. We already have a lot of Wine derived code used
in a large number of places, so it does not make sense to try to go
back and audit everything right now. We don't want to make it too much
of a painful process to do something simple like attribution.

If these suggestions seem like too much work or too much process, feel
free to disregard them. I will just keep pestering you if I see a
bunch of 'from Wine' svn commits with no attribution on them. =)
-- 
Steven Edwards

"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo

[James Tabor]

Here we go again!
LOL!
Do we get a vote in this?
LOL!

On Mon, Nov 17, 2008 at 8:41 AM, Steven Edwards [e-mail] wrote:
> On Mon, Nov 17, 2008 at 8:47 AM, gedmurphy [e-mail] wrote:
> > Steven, do Wine have any thoughts on this?
> > I only brought this up so it doesn't look like we're ignoring Wine's
> > requests.
>
> I propose we try to adopt a simple set of guidelines for how to handle
> Wine (or for that matter any other) code that is derived in ReactOS
> source files. The KEY thing is we want to keep it fun and not add a
> bunch of work otherwise no one will want to do it. If we need to do an
> audit later having some simple rules in place will save us some
> trouble. I think the rule of thumb should be something like

[James Tabor]

On Mon, Nov 17, 2008 at 7:40 AM, Aleksey Bragin [e-mail] wrote:
> On Nov 17, 2008, at 4:20 PM, gedmurphy wrote:
> > Steven has repeatedly asked for credit, and I quote:
> >
> > " Please add the appropriate copyright headers to the source file
> > stating the original author when implementing code based on third
> > party sources."
>
> There is one concern about it: how to find a proper author for a
> specific function he is copying? We can look at the top of the file,
> but there are a number of people listed there, possibly responsible
> for different parts of the file. Should he review all Wine commits
> and pick an author of the patch? What if the specific function was
> patched later by a few authors?
> > Is there a reason for not doing it this time?
> >
> > Secondly, why are things being implemented in stubs?
>
> There is same stuff in gdi32 stub.c having a few implemented
> functions, iirc. Not very good decision, but as long as people
> responsible remember about this.. and keep moving it to appropriate
> places, I don't mind.
>
> WBR,
> Aleksey.

Pointing out the source of the code is a correct practice... I will
start posting everything for now on and trust me wine'ies and code
weevils will still complain. See, they do not what this project to out
shine theirs and this is pushing the issue though their back door (Mr.
Edwards) which by the way is bring up these points. I need help and no
one is helping me and I'm doing this work for free and I do not want
to be known and this kills the code weevils pride. code weevils owe us
big time for creating this mess in the first place. They used the bock
door in the past and now pushing it through again and again. They
created the audit to kick us out of the game and now they see us again
as competition in a market place that has no buyers.

At this moment we have member that do not code very much making
decisions and creating changes to this project with out our input.
Tell me, is that fair?

For members of this project that do help, I give you great thanks! We need it!
James