Re: wininet: Add support for SSPI authentication for HTTP.
Alexandre Julliard wrote:
Robert Shearman [EMAIL PROTECTED] writes:
+if (strncmpiW(pszAuthValue, szBasic,
sizeof(szBasic)/sizeof(szBasic[0])-1))
+{
When using strncmp you need to also check that you reached the end of
the first string.
Hmm, it seems strncmpiW already does that for me so I'm a little confused:
int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n )
{
int ret = 0;
for ( ; n 0; n--, str1++, str2++)
if ((ret = tolowerW(*str1) - tolowerW(*str2)) || !*str1) break;
return ret;
}
+/* compare against last character to be set to avoid a race */
+if (HTTP_Base64Dec['/'] != 63)
+{
This won't avoid the race, you'll still get garbage if two threads get
here at the same time.
Good spot, I'll fix this.
--
Rob Shearman
Re: wininet: Add support for SSPI authentication for HTTP.
Juan Lang wrote: Hi Rob, +static UINT HTTP_DecodeBase64(LPCWSTR base64, LPSTR bin); Do we really have to keep proliferating base64 implementations? Could you not implement CryptStringToBinaryW instead? Should be mostly a copy-paste job.. CryptStringToBinaryW is a lot more complicated than just a base64 encoder function, so this is beyond the scope of what I was trying to implement. I'll make a note to replace this code by CryptStringToBinaryW if someone does implement it though. -- Rob Shearman
Re: mshtml: Change the binding flags when silent operation is requested by the DISPID_AMBIENT_SILENT property.
Alexandre Julliard wrote: Robert Shearman [EMAIL PROTECTED] writes: The binding flags should be have BINDF_SILENTOPERATION and BIND_NO_UI added. This breaks the tests: ../../../tools/runtest -q -P wine -M shdocvw.dll -T ../../.. -p shdocvw_test.exe.so webbrowser.c touch webbrowser.ok webbrowser.c:197: Test failed: unexpected call Invoke_AMBIENT_DLCONTROL make[2]: *** [webbrowser.ok] Error 1 I'm not sure how to fix that, so I'm going to have to drop the patch. -- Rob Shearman
Re: 2/4 setupapi: Don't return ERROR_WRONG_INF_STYLE when there is no version section.
On 5/18/07, Hans Leidekker [EMAIL PROTECTED] wrote: -Hans Changelog Don't return ERROR_WRONG_INF_STYLE when there is no version section. This is a change that is begging for a test case. -- James Hawkins
Re: 3/4 setupapi: Correctly handle an empty filename in SetupGetSourceFileLocationA.
On 5/18/07, Hans Leidekker [EMAIL PROTECTED] wrote: -Hans Changelog Correctly handle an empty filename in SetupGetSourceFileLocationA. Please add a test case. -- James Hawkins
Re: wininet: Add support for SSPI authentication for HTTP.
Alexandre Julliard wrote:
Robert Shearman [EMAIL PROTECTED] writes:
Hmm, it seems strncmpiW already does that for me so I'm a little confused:
int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n )
{
int ret = 0;
for ( ; n 0; n--, str1++, str2++)
if ((ret = tolowerW(*str1) - tolowerW(*str2)) || !*str1) break;
return ret;
}
It does if the string is shorter, but not if it's longer. It's OK if
you want to check that the string is a strict subset, but that's not
usually what you want.
In this case, it is. The Basic string should be followed by some
additional data which is parsed later.
--
Rob Shearman
Re: 3/4 setupapi: Correctly handle an empty filename in SetupGetSourceFileLocationA.
On Friday 18 May 2007 11:43:29 James Hawkins wrote: Correctly handle an empty filename in SetupGetSourceFileLocationA. Please add a test case. The BITS installer was enough of a test case for me here, but since you ask, I'll see what I can do. -Hans
Re: hhctrl.ocx: Parse HTML entities in the table of contents.
Alexandre Julliard wrote:
Robert Shearman [EMAIL PROTECTED] writes:
+/* hexadecimal entity */
+while ((*p = '0' *p = '9') || (*p = 'a' *p = 'f') ||
+ (*p = 'A' *p = 'F') || (*p == ';'))
+p++;
You should exit the loop at the first ';'. Also you have to increment
p first.
+while (isalpha(*p))
+p++;
You can't use ASCII ctype functions on Unicode chars.
Thanks, I'll send an updated patch which should address these comments.
+for (i = 0; i sizeof(char_refs)/sizeof(char_refs[0]); i++)
+if (p - start - 1 = sizeof(char_refs[i].name))
+{
+if (!strncmpW(char_refs[i].name, start + 1, p - start - 1))
+break;
+}
The sizeof check doesn't make much sense, especially inside the
loop. What you need is to check that you reached the end of the string
if the strncmpW succeeded.
Again, I'm not sure what you mean here. I know (start + 1) - p will not
have any nul's in it, therefore strncmpW will return non-zero if
char_refs[i].name has a nul in it before (p - start - 1) characters. I
wrote a small test program to prove this:
static const WCHAR str1[] = {'t','e','s','t',0};
static const WCHAR str2[] = {'t','e','s','t','s','t','r','i','n','g',0};
printf(strncmpW(str1, str2, sizeof(str2)/sizeof(str2[0])-1) =
%d\n, strncmpW(str1, str2, sizeof(str2)/sizeof(str2[0])-1));
It outputs:
strncmpW(str1, str2, sizeof(str2)/sizeof(str2[0])-1) = -115
So I'm what other cases I need to handle where strncmpW succeeds when it
shouldn't.
--
Rob Shearman
Re: wininet: Add support for SSPI authentication for HTTP.
Robert Shearman [EMAIL PROTECTED] writes:
Hmm, it seems strncmpiW already does that for me so I'm a little confused:
int strncmpiW( const WCHAR *str1, const WCHAR *str2, int n )
{
int ret = 0;
for ( ; n 0; n--, str1++, str2++)
if ((ret = tolowerW(*str1) - tolowerW(*str2)) || !*str1) break;
return ret;
}
It does if the string is shorter, but not if it's longer. It's OK if
you want to check that the string is a strict subset, but that's not
usually what you want.
--
Alexandre Julliard
[EMAIL PROTECTED]
Re: wininet: Add support for SSPI authentication for HTTP.
Robert Shearman [EMAIL PROTECTED] writes: In this case, it is. The Basic string should be followed by some additional data which is parsed later. Yes, but AFAICS it's still supposed to be a separate token, so you'd need to check for a token separator. I don't think Basically should be considered a match for Basic authentication. -- Alexandre Julliard [EMAIL PROTECTED]
Re: msxml3 [1/4]: Add initial implementations of IXMLElement and IXMLElementCollection
On Thu, May 17, 2007 at 04:40:06PM -0500, James Hawkins wrote: Changelog: * Add initial implementations of IXMLElement and IXMLElementCollection. I can't help thinking that it would have been cleaner to implement the IXML stuff using the IXMLDOM interfaces rather than straight on top of libxml. That way we only get one set of interfaces that depend on libxml. It may not be possible, but I think it would be worth a look. Huw. -- Huw Davies [EMAIL PROTECTED]
Re: ntdll: Protect RtlAllocateHeap and RtlReAllocateHeap against integer overflows with large values of size.
On Friday 18 May 2007 04:01:19 am Robert Shearman wrote: + ULONGLONG llret = (ULONGLONG)a + b; + if ((sizeof(SIZE_T) sizeof(ULONGLONG)) (llret 0x)) + return FALSE; WOuldn't this be more correct (as well as function when sizeof(SIZE_T) = sizeof(ULONGLONG)): SIZE_T res = a + b; return (res = a);
Re: ntdll: Protect RtlAllocateHeap and RtlReAllocateHeap against integer overflows with large values of size.
Chris Robinson wrote: On Friday 18 May 2007 04:01:19 am Robert Shearman wrote: +ULONGLONG llret = (ULONGLONG)a + b; +if ((sizeof(SIZE_T) sizeof(ULONGLONG)) (llret 0x)) +return FALSE; WOuldn't this be more correct (as well as function when sizeof(SIZE_T) = sizeof(ULONGLONG)): SIZE_T res = a + b; return (res = a); An example that would break using your logic: 2 + (-1) -- Rob Shearman
Re: ntdll: Protect RtlAllocateHeap and RtlReAllocateHeap against integer overflows with large values of size.
On Friday 18 May 2007 05:12:30 am you wrote: An example that would break using your logic: 2 + (-1) SIZE_T (if it follows standard size_t) is unsigned, though. Adding a negative wouldn't be possible.
Re: ntdll: Protect RtlAllocateHeap and RtlReAllocateHeap against integer overflows with large values of size.
Chris Robinson wrote: On Friday 18 May 2007 05:12:30 am you wrote: An example that would break using your logic: 2 + (-1) SIZE_T (if it follows standard size_t) is unsigned, though. Adding a negative wouldn't be possible. Yes, you're right. The second parameter should probably be SSIZE_T. -- Rob Shearman
Direct3D thread safety review
Hi,
After writing some tests for synchronisation in ddraw, I have new versions of
my thread safety patches for ddraw. I want to show them for some ideas /
comments before sending them in.
Basically windows ddraw seems to hold a DLL global lock whenever some code of
the DLL is executed even in functions that wouldn't need it(like AddRef, can
use interlocked addref), unrelated calls(2 different ddraw objects or
surfaces) or before checking errornous params.
As per Alexandre's comments, we don't want to copy all this insane locking
exactly(until we find an app that really needs this. Basically my 2 patches
handle it like this(for the ddraw object only - all others will follow):
* When a method needs locking, the lock is aquired after printing the trace,
but before checking error conditions(like windows).
* If a method doesn't need locking(e.g. unimplemented, or just a getter for
static info(GetCaps), or QI / AddRef / Release), then the lock is not held
Since ddraw, d3d8 and d3d9 need their own locking anyway, my plan is to
implement locking in them and assume synchronised calls in wined3d - ie no
wined3d locking. No 2 threads may call wined3d at the same time, and
ddraw/d3d8/d3d9 have to take care of that. That keeps the code simpler.
Attached is my test(threading.c), the locking for main and ddraw(merged it
accidentally - need to split it up again. The plan is to continue like that
for the rest of ddraw and d3d8/9
From e8a3a4850d6c21fffbb16af45a969bd92aa59e05 Mon Sep 17 00:00:00 2001
From: Stefan Doesinger [EMAIL PROTECTED]
Date: Fri, 18 May 2007 15:31:47 +0200
Subject: [PATCH] DDraw: Make the ddraw list lock a global dll lock
---
dlls/ddraw/ddraw_private.h |3 +++
dlls/ddraw/main.c | 22 +++---
2 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/dlls/ddraw/ddraw_private.h b/dlls/ddraw/ddraw_private.h
index 9845102..da16aba 100644
--- a/dlls/ddraw/ddraw_private.h
+++ b/dlls/ddraw/ddraw_private.h
@@ -80,6 +80,9 @@ extern ULONG WINAPI D3D7CB_DestroySwapChain(IWineD3DSwapChain *pSwapChain);
extern ULONG WINAPI D3D7CB_DestroyDepthStencilSurface(IWineD3DSurface *pSurface);
+/* Global critical section */
+CRITICAL_SECTION ddraw_cs;
+
/*
* IDirectDraw implementation structure
*/
diff --git a/dlls/ddraw/main.c b/dlls/ddraw/main.c
index 46c383a..6882ccf 100644
--- a/dlls/ddraw/main.c
+++ b/dlls/ddraw/main.c
@@ -59,15 +59,15 @@ WINED3DSURFTYPE DefaultSurfaceType = SURFACE_UNKNOWN;
/* DDraw list and critical section */
static struct list global_ddraw_list = LIST_INIT(global_ddraw_list);
-static CRITICAL_SECTION ddraw_list_cs;
-static CRITICAL_SECTION_DEBUG ddraw_list_cs_debug =
+CRITICAL_SECTION ddraw_cs;
+CRITICAL_SECTION_DEBUG ddraw_cs_debug =
{
-0, 0, ddraw_list_cs,
-{ ddraw_list_cs_debug.ProcessLocksList,
-ddraw_list_cs_debug.ProcessLocksList },
-0, 0, { (DWORD_PTR)(__FILE__ : ddraw_list_cs) }
+0, 0, ddraw_cs,
+{ ddraw_cs_debug.ProcessLocksList,
+ddraw_cs_debug.ProcessLocksList },
+0, 0, { (DWORD_PTR)(__FILE__ : ddraw_cs) }
};
-static CRITICAL_SECTION ddraw_list_cs = { ddraw_list_cs_debug, -1, 0, 0, 0, 0 };
+CRITICAL_SECTION ddraw_cs = { ddraw_cs_debug, -1, 0, 0, 0, 0 };
/***
*
@@ -319,9 +319,9 @@ DDRAW_Create(const GUID *guid,
list_init(This-surface_list);
-EnterCriticalSection(ddraw_list_cs);
+EnterCriticalSection(ddraw_cs);
list_add_head(global_ddraw_list, This-ddraw_list_entry);
-LeaveCriticalSection(ddraw_list_cs);
+LeaveCriticalSection(ddraw_cs);
This-decls = HeapAlloc(GetProcessHeap(), 0, 0);
if(!This-decls)
@@ -948,7 +948,7 @@ DllMain(HINSTANCE hInstDLL,
void
remove_ddraw_object(IDirectDrawImpl *ddraw)
{
-EnterCriticalSection(ddraw_list_cs);
+EnterCriticalSection(ddraw_cs);
list_remove(ddraw-ddraw_list_entry);
-LeaveCriticalSection(ddraw_list_cs);
+LeaveCriticalSection(ddraw_cs);
}
--
1.5.0.7
From 7585984da84cfde23774e5d3b1272a9a52759013 Mon Sep 17 00:00:00 2001
From: Stefan Doesinger [EMAIL PROTECTED]
Date: Fri, 18 May 2007 17:06:11 +0200
Subject: [PATCH] DDraw: Hold the lock in IDirectDrawX methods
---
dlls/ddraw/ddraw.c | 107 +---
dlls/ddraw/ddraw_private.h |3 -
dlls/ddraw/main.c | 42 +++--
3 files changed, 127 insertions(+), 25 deletions(-)
diff --git a/dlls/ddraw/ddraw.c b/dlls/ddraw/ddraw.c
index 2d8dc8a..ce591f1 100644
--- a/dlls/ddraw/ddraw.c
+++ b/dlls/ddraw/ddraw.c
@@ -97,11 +97,17 @@ IDirectDrawImpl_QueryInterface(IDirectDraw7 *iface,
TRACE((%p)-(%s,%p)\n, This, debugstr_guid(refiid), obj);
+/* Can change surface impl type */
+EnterCriticalSection(ddraw_cs);
+
/* According to COM docs, if
Re: [PATCH 3/4] msi: automation: Generalize list implementation.
Misha Koshelev [EMAIL PROTECTED] writes:
if (wFlags DISPATCH_PROPERTYGET) {
V_VT(pVarResult) = VT_I4;
-V_I4(pVarResult) = data-iCount;
+V_I4(pVarResult) = data-ulCount;
I committed this patch, but I can't resist pointing out that if you
had simply named the field count instead of using the (idiotic)
Hungarian notation, the code would be just as clear, and you wouldn't
need to rename it everywhere when making it unsigned...
--
Alexandre Julliard
[EMAIL PROTECTED]
ADVAPI32: service tests
While working on some advapi32.service functions I found that having some tests for those APIs could be very useful. However nbot having added completely new tests so far there are a few issues I feel intimidated about. The actual task of adding a new test file service.c does seem fairly straightforward to me, considering I can just take some other test file and copy the framework from it, adding the new service.c to the Makefile.in. However there are a few issues I see. First almost any test won't make much sense without at least one service installed and activated. As it is now there seem to be no services installed by default in a clean Wine install. So I guess I would also need an extra executable somehow that implements a basic service that could for instance communicate through a pipe with the service test. It could be as simple as an executable that implements all the necessary communication with the service manager and offers some sort of pipe loopback as main service activity. But how am I going to add such an extra executable to the make logic of the advapi32 test? Also under real Windows installing a service will in many cases fail since that requires administrative privileges for the current process and I'm not sure we can and want to enforce something like that especially for the unattended wine test service if that is still working. I would really appreciate some help about adding an extra executable to the creation to the advapi32 make logic so that meaningful tests could be done on Wine. Or maybe someone knows of a service that is going to be added to a standard Wine installation soon? Rolf Kalbermatter
Re: [PATCH 3/4] msi: automation: Generalize list implementation.
On Fri, 2007-05-18 at 20:15 +0200, Alexandre Julliard wrote:
Misha Koshelev [EMAIL PROTECTED] writes:
if (wFlags DISPATCH_PROPERTYGET) {
V_VT(pVarResult) = VT_I4;
-V_I4(pVarResult) = data-iCount;
+V_I4(pVarResult) = data-ulCount;
I committed this patch, but I can't resist pointing out that if you
had simply named the field count instead of using the (idiotic)
Hungarian notation, the code would be just as clear, and you wouldn't
need to rename it everywhere when making it unsigned...
So is the general recommendation for wine that we not use Hungarian
notation then?
Misha
RE: advapi32: Route all 4 EnumServicesStatus[Ex] calls to a single stubto avoid code duplication
Paul Chitescu wrote: Changelog: advapi32: Route all 4 EnumServicesStatus[Ex] calls to a single stub to avoid code duplication Most of the actual enumeration is common code so it makes sense to implement it only once. This is a possible approach. I had intended to just make use of EnumServicesStatusExW from all other functions with some translation in them. Your approach avoids translations at the cost of making the common enumeration function quite a bit more complicated. I'm not sure which one I like more. Rolf Kalbermatter
Re: [PATCH 3/4] msi: automation: Generalize list implementation.
Am Freitag 18 Mai 2007 23:17 schrieb Misha Koshelev:
On Fri, 2007-05-18 at 20:15 +0200, Alexandre Julliard wrote:
Misha Koshelev [EMAIL PROTECTED] writes:
if (wFlags DISPATCH_PROPERTYGET) {
V_VT(pVarResult) = VT_I4;
-V_I4(pVarResult) = data-iCount;
+V_I4(pVarResult) = data-ulCount;
I committed this patch, but I can't resist pointing out that if you
had simply named the field count instead of using the (idiotic)
Hungarian notation, the code would be just as clear, and you wouldn't
need to rename it everywhere when making it unsigned...
So is the general recommendation for wine that we not use Hungarian
notation then?
Alexandre usually calls it Hungarian Line Noise, I guess that means no.
Re: [PATCH 3/4] msi: automation: Generalize list implementation.
Misha Koshelev wrote: So is the general recommendation for wine that we not use Hungarian notation then? Misha IMHO, a prefix is of little value if it merely echos the declared type of the variable it represents, especially in a small-scope situation. To be of value, a prefix needs to convey other information, such as implying the purpose of the variable, or acting as a grouping element for a set of related variables. -- Andy.
Re: RegDeleteTree [6]
Hi Stefan,
+if (!lpszName) {
+ret = ERROR_NOT_ENOUGH_MEMORY;
+ goto cleanup;
+}
Now you're mixing tabs and spaces (here and elsewhere.) Please honor the
existing formatting.
--Juan
Yahoo!
oneSearch: Finally, mobile search
that gives answers, not web links.
http://mobile.yahoo.com/mobileweb/onesearch?refer=1ONXIC
