Re: [ros-dev] [ros-diffs] [sginsberg] 36773: - Unbreak GetTextFaceA/W, broken since 28730 by GreatLord

[gedmurphy]

Why do you need to publically state it was broken and who broke it?
Does it make you feel more important and help to boost your ego?

-----Original Message-----
From: [e-mail] [mailto:[e-mail]] On Behalf Of [e-mail]
Sent: 16 October 2008 21:02
To: [e-mail]
Subject: [ros-diffs] [sginsberg] 36773: - Unbreak GetTextFaceA/W, broken since 28730 by GreatLord

Author: sginsberg
Date: Thu Oct 16 15:02:22 2008
New Revision: 36773

URL: http://svn.reactos.org/svn/reactos?rev=36773&view=rev
Log:
- Unbreak GetTextFaceA/W, broken since 28730 by GreatLord

Modified:
    trunk/reactos/dll/win32/gdi32/objects/text.c

Modified: trunk/reactos/dll/win32/gdi32/objects/text.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/gdi32/objects/tex...
==============================================================================
--- trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] (original)
+++ trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] Thu Oct 16 15:02:22 2008
@@ -358,20 +358,24 @@
  * @implemented
  */
 INT
-STDCALL
+WINAPI
 GetTextFaceW(HDC hDC,
-             int nCount,
-             LPWSTR	lpFaceName)
-{
-    INT retValue = 0;
-    if ((!lpFaceName) || (nCount))
-    {
-        retValue = NtGdiGetTextFaceW(hDC,nCount,lpFaceName,0);
+             INT nCount,
+             PWSTR pFaceName)
+{
+    INT retValue;
+
+    if ((pFaceName && nCount) ||
+        !(pFaceName && nCount))
+    {
+        retValue = NtGdiGetTextFaceW(hDC, nCount, pFaceName, 0);
     }
     else
     {
         SetLastError(ERROR_INVALID_PARAMETER);
-    }
+        retValue = 0;
+    }
+
     return retValue;
 }

[Timo Kreuzer]

Especially if the "fix" is no way better ;-)

gedmurphy schrieb:
> Why do you need to publically state it was broken and who broke it?
> Does it make you feel more important and help to boost your ego?
>
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of [e-mail]
> Sent: 16 October 2008 21:02
> To: [e-mail]
> Subject: [ros-diffs] [sginsberg] 36773: - Unbreak GetTextFaceA/W, broken since 28730 by GreatLord
>
> Author: sginsberg
> Date: Thu Oct 16 15:02:22 2008
> New Revision: 36773
>
> URL: http://svn.reactos.org/svn/reactos?rev=36773&view=rev
> Log:
> Unbreak GetTextFaceA/W, broken since 28730 by GreatLord
>
> Modified:
>     trunk/reactos/dll/win32/gdi32/objects/text.c
>
> Modified: trunk/reactos/dll/win32/gdi32/objects/text.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/gdi32/objects/tex...
> ==============================================================================
> --- trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] (original)
> +++ trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] Thu Oct 16 15:02:22 2008
> @@ -358,20 +358,24 @@
> @implemented
>
> */
>  INT
> -STDCALL
> +WINAPI
>  GetTextFaceW(HDC hDC,
>         int nCount,
>         LPWSTR	lpFaceName)
>
> -{
> INT retValue = 0;
> if ((!lpFaceName) || (nCount))
> {
>    retValue = NtGdiGetTextFaceW(hDC,nCount,lpFaceName,0);
>         INT nCount,
>         PWSTR pFaceName)
>
> +{
> INT retValue;
> if ((pFaceName && nCount) ||
>    !(pFaceName && nCount))
> {
>    retValue = NtGdiGetTextFaceW(hDC, nCount, pFaceName, 0);
> }
>    else
>    {
>        SetLastError(ERROR_INVALID_PARAMETER);
> }
>    retValue = 0;
> }
> return retValue;
>
> }
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Matthew Therault]

Agreed.... Everyone makes mistakes.  You should point out mistakes in the
least personal matter. People don't learn if you attack them personally, and
they tend to shut off towards others.

On Thu, Oct 16, 2008 at 5:08 PM, gedmurphy [e-mail] wrote:
> Why do you need to publically state it was broken and who broke it?
> Does it make you feel more important and help to boost your ego?
>
> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]]
> On Behalf Of [e-mail]
> Sent: 16 October 2008 21:02
> To: [e-mail]
> Subject: [ros-diffs] [sginsberg] 36773: - Unbreak GetTextFaceA/W, broken
> since 28730 by GreatLord
>
> Author: sginsberg
> Date: Thu Oct 16 15:02:22 2008
> New Revision: 36773
>
> URL: http://svn.reactos.org/svn/reactos?rev=36773&view=rev
> Log:
> Unbreak GetTextFaceA/W, broken since 28730 by GreatLord
>
> Modified:
>    trunk/reactos/dll/win32/gdi32/objects/text.c
>
> Modified: trunk/reactos/dll/win32/gdi32/objects/text.c
> URL:
> http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/gdi32/objects/tex...
>
> ==============================================================================
> --- trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] (original)
> +++ trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] Thu Oct 16
> 15:02:22 2008
> @@ -358,20 +358,24 @@
> @implemented
>
> */
>  INT
> -STDCALL
> +WINAPI
>  GetTextFaceW(HDC hDC,
>         int nCount,
>         LPWSTR    lpFaceName)
>
> -{
> INT retValue = 0;
> if ((!lpFaceName) || (nCount))
> {
>    retValue = NtGdiGetTextFaceW(hDC,nCount,lpFaceName,0);
>         INT nCount,
>         PWSTR pFaceName)
>
> +{
> INT retValue;
> if ((pFaceName && nCount) ||
>    !(pFaceName && nCount))
> {
>    retValue = NtGdiGetTextFaceW(hDC, nCount, pFaceName, 0);
> }
>   else
>   {
>       SetLastError(ERROR_INVALID_PARAMETER);
> }
>    retValue = 0;
> }
> return retValue;
>
> }
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev
-- 
Two men walk into a bar....... Third one ducks....

Think about it, it will come to you.  :P

[WaxDragon]

On Thu, Oct 16, 2008 at 5:08 PM, gedmurphy [e-mail] wrote:
> Why do you need to publically state it was broken and who broke it?
> Does it make you feel more important and help to boost your ego?

*WE* are a team, *WE* broke it.

[Timo Kreuzer]

Ok, I need to defend Magnus here, I guess (thanks KJK::Hyperion for
noticing)

Actually, Magnus did the correct check and *WE* (*cough*) broke it.
Please check rostests/apitests/gdi32api/tests/GetTextFace.c

I refuse to put logic tables here. It was all correct!

"If it ain't broke, fix it till it is...."

Timo

WaxDragon schrieb:
> On Thu, Oct 16, 2008 at 5:08 PM, gedmurphy [e-mail] wrote:
> > Why do you need to publically state it was broken and who broke it?
> > Does it make you feel more important and help to boost your ego?
>
> *WE* are a team, *WE* broke it.
> _______________________________________________
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[KJK::Hyperion]

Timo Kreuzer wrote:
> Ok, I need to defend Magnus here, I guess (thanks KJK::Hyperion for
> noticing)

here's how I tested the behavior of GetTextFaceW:

#define WIN32_LEAN_AND_MEAN
#define STRICT
#define NOMINMAX

#include <windows.h>

#include <stdio.h>
#include <tchar.h>

int main()
{
    HDC hdc = CreateCompatibleDC(NULL);

TCHAR buf[1];

int c = ARRAYSIZE(buf);
    LPTSTR lpName = buf;

SetLastError(1); _tprintf(_T("GetTextFace(hdc, %d, %p) -> %d,
GetLastError() -> %lu\n"), 0, NULL,   GetTextFace(hdc, 0, NULL),
GetLastError());

SetLastError(1); _tprintf(_T("GetTextFace(hdc, %d, %p) -> %d,
GetLastError() -> %lu\n"), 0, lpName, GetTextFace(hdc, 0, lpName),
GetLastError());

SetLastError(1); _tprintf(_T("GetTextFace(hdc, %d, %p) -> %d,
GetLastError() -> %lu\n"), c, lpName, GetTextFace(hdc, c, lpName),
GetLastError());

SetLastError(1); _tprintf(_T("GetTextFace(hdc, %d, %p) -> %d,
GetLastError() -> %lu\n"), c, NULL,   GetTextFace(hdc, c, NULL),
GetLastError());
}

// EOF

And this is the output:

GetTextFace(hdc, 0, 00000000) -> 7, GetLastError() -> 1
GetTextFace(hdc, 0, 0021FD70) -> 0, GetLastError() -> 1
GetTextFace(hdc, 1, 0021FD70) -> 1, GetLastError() -> 1
GetTextFace(hdc, 1, 00000000) -> 7, GetLastError() -> 1

(7 is sizeof("System"))

When lpName is NULL, the API always return the required size, ignoring
c. If a buffer is specified, it must be at least 1 character long (0
characters won't even fit an empty string), otherwise failure. Hence,
the success conditional is !(lpName && !c), i.e. (!lpName || c), which
is what it had been all along

[James Tabor]

OMG, now I'm getting dragged into this!

On Thu, Oct 16, 2008 at 7:45 PM, KJK::Hyperion [e-mail] wrote:
> When lpName is NULL, the API always return the required size, ignoring
> c. If a buffer is specified, it must be at least 1 character long (0
> characters won't even fit an empty string), otherwise failure. Hence,
> the success conditional is !(lpName && !c), i.e. (!lpName || c), which
> is what it had been all along

Okay Vista may not set GdiSetLastError but XP does! Did anyone bother
to check GetTextFaceAliasW? If anyone does please move it to
objects/text.c too.

Thanks,
James

[James Tabor]

Hi,
I simplified the program a bit~ 8^D

The compiler does things backwards sometimes. If this was a m68k
system it would have worked the first time.

Sorry,
James

// gcc -mconsole -mwindows gtfw.c -o gtfw.exe
//
// GetTextFaceW(hdc, 0, 00000000) -> 7,GetLastError() -> 1
// GetTextFaceW(hdc, 0, 0022FF52) -> 0,GetLastError() -> 87
// GetTextFaceW(hdc, -10000, 0022FF52) -> 0,GetLastError() -> 87
// GetTextFaceW(hdc, 1, 0022FF52) -> 1, GetLastError() -> 1
// GetTextFaceW(hdc, 1, 00000000) -> 7,GetLastError() -> 1
#include <windows.h>
#include <stdio.h>

int main()
{
    HDC hdc = CreateCompatibleDC(NULL);

WCHAR buf[1];
        int ret;
    int c = 1;
    LPWSTR lpName = buf;

SetLastError(1);
        ret = GetTextFaceW(hdc, 0, NULL);
        printf(("GetTextFaceW(hdc, %d, %p) -> %d,GetLastError() ->
%lu\n"), 0, NULL, ret, GetLastError());

SetLastError(1);
        ret = GetTextFaceW(hdc, 0, lpName);
        printf(("GetTextFaceW(hdc, %d, %p) -> %d,GetLastError() ->
%lu\n"), 0, lpName, ret,GetLastError());

SetLastError(1);
        ret = GetTextFaceW(hdc, -10000, lpName);
        printf(("GetTextFaceW(hdc, %d, %p) -> %d,GetLastError() ->
%lu\n"), -10000, lpName, ret,GetLastError());

SetLastError(1);
        ret = GetTextFaceW(hdc, c, lpName);
        printf(("GetTextFaceW(hdc, %d, %p) -> %d, GetLastError() ->
%lu\n"), c, lpName, ret,GetLastError());

SetLastError(1);
        ret = GetTextFaceW(hdc, c, NULL);
        printf(("GetTextFaceW(hdc, %d, %p) -> %d,GetLastError() ->
%lu\n"), c, NULL,  ret,GetLastError());
}

// EOF

[Aleksey Bragin]

Stating the revision number is enough (and useful). Stating the  
actual author is redundant, but might feed someone's ego (though I  
think it was never taken too close to heart).

WBR,
Aleksey Bragin.

On Oct 17, 2008, at 1:08 AM, gedmurphy wrote:
> Why do you need to publically state it was broken and who broke it?
> Does it make you feel more important and help to boost your ego?
>
> -----Original Message-----
> From: [e-mail] [mailto:ros-diffs- 
> [e-mail]] On Behalf Of [e-mail]
> Sent: 16 October 2008 21:02
> To: [e-mail]
> Subject: [ros-diffs] [sginsberg] 36773: - Unbreak GetTextFaceA/W,  
> broken since 28730 by GreatLord
>
> Author: sginsberg
> Date: Thu Oct 16 15:02:22 2008
> New Revision: 36773
>
> URL: http://svn.reactos.org/svn/reactos?rev=36773&view=rev
> Log:
> Unbreak GetTextFaceA/W, broken since 28730 by GreatLord
>
> Modified:
>     trunk/reactos/dll/win32/gdi32/objects/text.c
>
> Modified: trunk/reactos/dll/win32/gdi32/objects/text.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/dll/win32/ 
> gdi32/objects/text.c?rev=36773&r1=36772&r2=36773&view=diff
> ====================================================================== 
> ========
> --- trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1]  
> (original)
> +++ trunk/reactos/dll/win32/gdi32/objects/text.c [iso-8859-1] Thu  
> Oct 16 15:02:22 2008
> @@ -358,20 +358,24 @@
> @implemented
>
> */
>  INT
> -STDCALL
> +WINAPI
>  GetTextFaceW(HDC hDC,
>         int nCount,
>         LPWSTR	lpFaceName)
>
> -{
> INT retValue = 0;
> if ((!lpFaceName) || (nCount))
> {
>    retValue = NtGdiGetTextFaceW(hDC,nCount,lpFaceName,0);
>         INT nCount,
>         PWSTR pFaceName)
>
> +{
> INT retValue;
> if ((pFaceName && nCount) ||
>    !(pFaceName && nCount))
> {
>    retValue = NtGdiGetTextFaceW(hDC, nCount, pFaceName, 0);
> }
>    else
>    {
>        SetLastError(ERROR_INVALID_PARAMETER);
> }
>    retValue = 0;
> }
> return retValue;
>
> }
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev