Re: ntdll: Support pinning module refcount with LdrAddRefDll()
On 10/14/2013 05:21, Dmitry Timoshkov wrote: Nikolay Sivov wrote: +FreeLibrary(mod); Please add the tests for FreeLibrary return value. Makes sense, thanks.
Re: wmvcore: Stub implementation of IWMMetadataEditor interface
On 10/10/2013 14:58, Jeff Latimer wrote:
---
dlls/wmvcore/Makefile.in| 2 +-
dlls/wmvcore/wmvcore_main.c | 100
+++-
include/wmsdkidl.idl| 11 ++---
3 files changed, 105 insertions(+), 8 deletions(-)
+typedef struct MetadataEditorImpl {
+IWMMetadataEditor MetadataEditor_iface;
+LONG ref;
+} MetadataEditorImpl;
+static inline MetadataEditorImpl *impl_from_MetadataEditor(IWMMetadataEditor
*iface)
Please follow naming convetions for interfaces.
+HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface)
+{
+FIXME("iface=%p\n", iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}
+HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface)
+{
+FIXME("iface=%p\n", iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}
This looks totally wrong.
Also please keep implementation functions order and interface methods
order the same.
Re: xmllite: Use BOOL type where appropriate
On 10/10/2013 00:36, Frédéric Delanoy wrote:
---
dlls/xmllite/reader.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/dlls/xmllite/reader.c b/dlls/xmllite/reader.c
index 0a4423c..a216951 100644
--- a/dlls/xmllite/reader.c
+++ b/dlls/xmllite/reader.c
@@ -726,7 +726,7 @@ static void readerinput_grow(xmlreaderinput *readerinput,
int length)
}
}
-static inline int readerinput_is_utf8(xmlreaderinput *readerinput)
+static inline BOOL readerinput_is_utf8(xmlreaderinput *readerinput)
{
static char startA[] = {'<','?'};
static char commentA[] = {'<','!'};
@@ -953,7 +953,7 @@ static void reader_skipn(xmlreader *reader, int n)
}
}
-static inline int is_wchar_space(WCHAR ch)
+static inline BOOL is_wchar_space(WCHAR ch)
{
return ch == ' ' || ch == '\t' || ch == '\r' || ch == '\n';
}
@@ -1055,7 +1055,7 @@ static HRESULT reader_parse_versioninfo(xmlreader *reader)
}
/* ([A-Za-z0-9._] | '-') */
-static inline int is_wchar_encname(WCHAR ch)
+static inline BOOL is_wchar_encname(WCHAR ch)
{
return ((ch >= 'A' && ch <= 'Z') ||
(ch >= 'a' && ch <= 'z') ||
@@ -1269,7 +1269,7 @@ static HRESULT reader_parse_comment(xmlreader *reader)
}
/* [2] Char ::= #x9 | #xA | #xD | [#x20-#xD7FF] | [#xE000-#xFFFD] | [#x1-#x10] */
-static inline int is_char(WCHAR ch)
+static inline BOOL is_char(WCHAR ch)
{
return (ch == '\t') || (ch == '\r') || (ch == '\n') ||
(ch >= 0x20 && ch <= 0xd7ff) ||
@@ -1279,7 +1279,7 @@ static inline int is_char(WCHAR ch)
}
/* [13] PubidChar ::= #x20 | #xD | #xA | [a-zA-Z0-9] | [-'()+,./:=?;!*#@$_%] */
-static inline int is_pubchar(WCHAR ch)
+static inline BOOL is_pubchar(WCHAR ch)
{
return (ch == ' ') ||
(ch >= 'a' && ch <= 'z') ||
@@ -1292,7 +1292,7 @@ static inline int is_pubchar(WCHAR ch)
(ch == '_') || (ch == '\r') || (ch == '\n');
}
-static inline int is_namestartchar(WCHAR ch)
+static inline BOOL is_namestartchar(WCHAR ch)
{
return (ch == ':') || (ch >= 'A' && ch <= 'Z') ||
(ch == '_') || (ch >= 'a' && ch <= 'z') ||
@@ -1312,7 +1312,7 @@ static inline int is_namestartchar(WCHAR ch)
}
/* [4 NS] NCName ::= Name - (Char* ':' Char*) */
-static inline int is_ncnamechar(WCHAR ch)
+static inline BOOL is_ncnamechar(WCHAR ch)
{
return (ch >= 'A' && ch <= 'Z') ||
(ch == '_') || (ch >= 'a' && ch <= 'z') ||
@@ -1336,7 +1336,7 @@ static inline int is_ncnamechar(WCHAR ch)
(ch >= 0xfdf0 && ch <= 0xfffd);
}
-static inline int is_namechar(WCHAR ch)
+static inline BOOL is_namechar(WCHAR ch)
{
return (ch == ':') || is_ncnamechar(ch);
}
I don't actually see what this will achieve, but I see such patches are
accepted. Is it a new style rule?
Re: [1/3] xmllite: Use buffer offset instead of pointers
On 10/8/2013 10:56, Alexandre Julliard wrote: Nikolay Sivov writes: On 10/6/2013 19:06, Nikolay Sivov wrote: It's normal to grow destination buffer, in this case all stored pointers will be trashed. This patch uses offsets from start of a buffer instead. Hi, Alexandre. Patches list shows a build failure for this one, and I don't see any failures on testbot for it so assuming something it fails on your machine. So what's the exact failure you get? It's compiler warnings, you have several uninitialized or unused variables. Please review your changes carefully. Okay, will do. Strangely I see no warnings here, but that's probably a different gcc version.
Re: [1/3] xmllite: Use buffer offset instead of pointers
On 10/6/2013 19:06, Nikolay Sivov wrote: It's normal to grow destination buffer, in this case all stored pointers will be trashed. This patch uses offsets from start of a buffer instead. Hi, Alexandre. Patches list shows a build failure for this one, and I don't see any failures on testbot for it so assuming something it fails on your machine. So what's the exact failure you get?
Re: kernel32: Add tests for job objects (try 3)
On 10/2/2013 12:58, Andrew Cook wrote:
+if(!pCreateJobObjectW) {
+win_skip("No job object support\n");
+return;
+}
Once you checked that I suppose you don't need to check for
pIsProcessInJob being null, or it still could happen?
+if (option && strcmp(option, "wait") == 0) {
+/* for job object tests */
+Sleep(3000);
+return;
+}
Is it really necessary?
Re: [2/3] kernel32/tests: test CopyFileEx callback and cancellation (resend)
On 9/30/2013 10:10, Daniel Jeliński wrote:
2013/9/30 Nikolay Sivov <mailto:bungleh...@gmail.com>>
On 9/30/2013 00:51, Daniel Jeliński wrote:
+struct progress_list {
+const DWORD progress_retval_init; /* value to return
from progress routine */
+const BOOL cancel_init;/* value to set Cancel
flag to */
+const DWORD progress_retval_end; /* value to return
from progress routine */
+const BOOL cancel_end; /* value to set Cancel
flag to */
+const DWORD progress_count;/* number of times
progress is invoked */
+const BOOL copy_retval;/* expected CopyFileEx
result */
+const DWORD lastError; /* expected CopyFileEx
error code */
+} ;
I don't see a point making them 'const'.
I'm matching the formatting of existing code:
http://source.winehq.org/source/dlls/kernel32/tests/file.c#L65
Also, what's the point of not making them const?
+static DWORD WINAPI progress(LARGE_INTEGER TotalFileSize,
+LARGE_INTEGER TotalBytesTransferred,
+LARGE_INTEGER StreamSize,
+LARGE_INTEGER StreamBytesTransferred,
+DWORD dwStreamNumber,
+DWORD dwCallbackReason,
+HANDLE hSourceFile,
+HANDLE hDestinationFile,
+LPVOID lpData)
+{
+progressInvoked++;
Please pass all globals as context data with lpData, and please
use 'void*' instead of LPVOID.
Good point about lpData. Still, does that make the patch invalid?
It's not black or white, I mentioned what will be nice to do about it to
make it more compact and self-contained.
It doesn't mean it's invalid, it's just an obvious thing that will make
it better.
Why didn't you mention that on the first review?
Because sometimes I stop reading further after I see major problems.
Also, any comments on patch 3?
Same thing, you could easily pack everything to a single struct and pass
it using this context pointer.
It could also be made more compact using a single helper to compar
COPYFILE2_MESSAGE value,
instead of duplicating it for every message type.
Re: [2/3] kernel32/tests: test CopyFileEx callback and cancellation (resend)
On 9/30/2013 00:51, Daniel Jeliński wrote:
+struct progress_list {
+const DWORD progress_retval_init; /* value to return from progress
routine */
+const BOOL cancel_init;/* value to set Cancel flag to */
+const DWORD progress_retval_end; /* value to return from progress
routine */
+const BOOL cancel_end; /* value to set Cancel flag to */
+const DWORD progress_count;/* number of times progress is invoked
*/
+const BOOL copy_retval;/* expected CopyFileEx result */
+const DWORD lastError; /* expected CopyFileEx error code */
+} ;
I don't see a point making them 'const'.
+static DWORD WINAPI progress(LARGE_INTEGER TotalFileSize,
+LARGE_INTEGER TotalBytesTransferred,
+LARGE_INTEGER StreamSize,
+LARGE_INTEGER StreamBytesTransferred,
+DWORD dwStreamNumber,
+DWORD dwCallbackReason,
+HANDLE hSourceFile,
+HANDLE hDestinationFile,
+LPVOID lpData)
+{
+progressInvoked++;
Please pass all globals as context data with lpData, and please use
'void*' instead of LPVOID.
Re: (try 5)[1/4] imm32: Move thread data from TLSEntry to an internal list
On 9/25/2013 16:03, Aric Stewart wrote:
+if (!thread_data->defaultContext)
+{
+HIMC defaultContext;
+LeaveCriticalSection(&threaddata_cs);
+defaultContext = ImmCreateContext();
+thread_data = IMM_GetThreadData(0);
Why do you need to unlock/lock around ImmCreateContext()? Is it related
to SendMessage() it calls?
Re: (try 5) [2/4] imm32: do not let ImmDestroyContext destroy any default contexts
On 9/25/2013 16:03, Aric Stewart wrote: defaultContext = ImmCreateContext(); +if (defaultContext) +((InputContextData*)defaultContext)->threadDefault = TRUE; I think a nicer way is to let ImmCreateContext() set this attribute.
Re: [1/4] ntdll/tests: Add 0-length read tests for a disk file.
On 09/18/2013 10:53 AM, Dmitry Timoshkov wrote: --- dlls/ntdll/tests/file.c | 41 + 1 file changed, 41 insertions(+) diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 120fdac..5ae605b 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -1985,6 +1985,22 @@ static void test_read_write(void) bytes = 0xdeadbeef; SetLastError(0xdeadbeef); +ret = ReadFile(INVALID_HANDLE_VALUE, buf, 0,&bytes, NULL); +todo_wine +ok(!ret, "ReadFile should fail\n"); +todo_wine +ok(GetLastError() == ERROR_INVALID_HANDLE, "expected ERROR_INVALID_HANDLE, got %d\n", GetLastError()); +ok(bytes == 0, "bytes %u\n", bytes); That's again a test for kernel32 calls, placed in a wrong file.
Re: oledb32: Implement IErrorRecord GetBasicErrorInfo
On 09/18/2013 09:47 AM, Alistair Leslie-Hughes wrote:
+LIST_FOR_EACH_ENTRY(cursor,&This->errors, struct ErrorEntry, entry)
+{
+if(nCnt == ulRecordNum)
+{
+*pErrorInfo = cursor->info;
+break;
+}
+}
+
This looks wrong, cause you never update nCnt. Also it doesn't look
appropriate to use list to store these records - methods provide
index-based access, so array fits better.
entry->lookupID = dwDynamicErrorID;
Any idea how this field could be used later? I don't where it could be
returned. And 'dwLookupID' parameter of 'AddErrorRecord' is never used,
that looks suspicious.
Re: [1/9] Fix expected values in various messages.
On 09/15/2013 04:25 PM, Akihiro Sagawa wrote: hr = IUnknown_QueryInterface(reader_input,&IID_IStream, (void**)&stream2); -ok(hr == E_NOINTERFACE, "Expected S_OK, got %08x\n", hr); +ok(hr == E_NOINTERFACE, "Expected E_NOINTERFACE, got %08x\n", hr); As Henri said, it's not that useful to see expected value, this one is a copy-paste typo of course and should be fixed. Printing actual value is enough, so something like "got 0x%08x\n" is ok.
Re: [2/2] kernel32: implement callback and cancellation in CopyFileEx
On 09/14/2013 11:46 PM, Daniel Jeliński wrote: 2013/9/14 Nikolay Sivov <mailto:nsi...@codeweavers.com>> Could you implement necessary part of CopyFile2 instead, and use it in CopyFileEx? I think CopyFile2 has everything to do that. I suppose I could, but then, CopyFile2 uses a different callback than CopyFileEx: CopyFile2: http://msdn.microsoft.com/en-us/library/windows/desktop/hh449407%28v=vs.85%29.aspx CopyFileEx: http://msdn.microsoft.com/en-us/library/windows/desktop/aa363854%28v=vs.85%29.aspx Not sure how to handle that. Any hints? You could add CopyFile2 style callback function in our code, pass everything you need as 'pvCallbackContext', that could be CopyFileEx callback pointer or pointer to a structure that contains pointer to user callback (and anything else you might need). I didn't look deep at this, but looks like CopyFile2 callback is called more often than CopyFileEx expects, this could be easily implement by filtering unwanted events in your CopyFile2 callback (that will reside in kernel32).
Re: [2/2] kernel32: implement callback and cancellation in CopyFileEx
On 09/14/2013 03:25 AM, Daniel Jeliński wrote: Could you implement necessary part of CopyFile2 instead, and use it in CopyFileEx? I think CopyFile2 has everything to do that.
Re: kernel32/tests: Add initial CreateFile2 tests based on the CreateFileW tests (try 2)
On 9/13/2013 22:26, André Hentschel wrote:
+if (!pCreateFile2)
+{
+win_skip("CreateFile2 is missing\n");
+return;
+}
Should be skip() so it'll show up when running with wine. Not sure how
important it is though.
Re: kernel32/tests: Add initial CreateFile2 tests based on the CreateFileW tests (try 2)
On 9/13/2013 22:26, André Hentschel wrote: Sorry, ignore my comment. I missed that we actually have this call already in wine.
Re: [PATCH 3/3] riched20: Add UTF8 support for EM_SETTEXTEX. (try 6)
On 9/13/2013 15:05, Jactry Zeng wrote: bUnicode = !bRtf && pStruct->codepage == CP_UNICODE; +bUTF8 = (lParam && (!strncmp((char *)lParam, utf8_bom, 3))); What will happen if both of these are true? This needs a test with BOM and 'codepage' set to 1200.
Re: [PATCH 1/3] riched20: Use codepage in ME_ToUnicode. (try 5)
On 9/13/2013 13:27, Jactry Zeng wrote: Re-based to latest git. Does it actually accept any other value besides CP_ACP or 1200? Documentation is unclear here, and makes impression that only two these values are valid.
Re: ntdll: Add a file access test.
On 9/13/2013 12:16, Dmitry Timoshkov wrote: Nikolay Sivov wrote: It looks like it belongs to kernel32/tests. Since actual access checks are done by ntdll APIs I believe that ntdll/tests is appropriate place, kernel32 file APIs are just wrappers around the tested functionality. It doesn't matter what it uses internally. You're testing kernel32 calls.
Re: ntdll: Add a file access test.
On 9/13/2013 11:20, Dmitry Timoshkov wrote: This test passes under Wine and shows that ReadFile after CreateFile(GENERIC_WRITE) is really supposed to fail. --- dlls/ntdll/tests/file.c | 82 + 1 file changed, 82 insertions(+) It looks like it belongs to kernel32/tests.
Re: [2/2] oledb32: Implement IDataSourceLocator get/put hWnd (resend)
On 9/10/2013 13:30, Alistair Leslie-Hughes wrote: Hi, Changelog: oledb32: Implement IDataSourceLocator get/put hWnd Best Regards Alistair Leslie-Hughes Hi, Alistair. This is wrong, interface definition should be fixed as well, HWND parameter can't be LONG: --- [id(0x6002), propget] HRESULT hWnd([out, retval] long* phwndParent); [id(0x6002), propput] HRESULT hWnd([in] long phwndParent); --- PSDK header has COMPATIBLE_LONG thing to make it work on win64.
Re: [PATCH 1/3] riched20: Use codepage in ME_ToUnicode. (try 2)
On 9/4/2013 21:16, Jactry Zeng wrote: Hi Nikolay, 2013/9/5 Nikolay Sivov <mailto:bungleh...@gmail.com>> > > On 9/4/2013 07:17, Jactry Zeng wrote: >> >> -ME_EndToUnicode(unicode, wszText); >> +ME_EndToUnicode(unicode ? 1200 : CP_ACP, wszText); > > It's still ugly to use magic numbers like that, in a similar situation I had some time > ago I use ~0 to mark WCHAR data encoding that does need special handling. > Even with a value like ~0 it needs to be documented in code comments. > Thanks for your review! But I still can't understand it. :( Can you show me a commit as an example. What I mean is it's better to use something more neutral to mark an absence of codepage. In my case it was like that http://source.winehq.org/git/wine.git/commit/320d419be1fe713c50eac7d53e4dc52481fad8b7 Thank you! (sorry for forgeting cc wine-devel) -- Regards, Jactry Zeng
Re: [PATCH 1/3] riched20: Use codepage in ME_ToUnicode. (try 2)
On 9/4/2013 07:17, Jactry Zeng wrote: -ME_EndToUnicode(unicode, wszText); +ME_EndToUnicode(unicode ? 1200 : CP_ACP, wszText); It's still ugly to use magic numbers like that, in a similar situation I had some time ago I use ~0 to mark WCHAR data encoding that does need special handling. Even with a value like ~0 it needs to be documented in code comments.
Re: Sequences in a loop
On 8/30/2013 16:33, matyapiro31 wrote: I found many wine sources use not a pointer,but a sequence . for example: krnl386.exe16/dma.c: for(i=0,p=(char*)DMA_CurrentBaseAddress[channel];i I guess by sequences you mean arrays? Should I change them? Change them to what? And what will it solve?
Re: oleaut32: Add ICreateTypeInfo2::SetVarName implementation
On 8/30/2013 16:28, Piotr Caban wrote:
---
dlls/oleaut32/tests/typelib.c | 13 +++--
dlls/oleaut32/typelib.c | 13 +++--
2 files changed, 22 insertions(+), 4 deletions(-)
ITypeInfoImpl *This = info_impl_from_ICreateTypeInfo2(iface);
-FIXME("%p %u %s - stub\n", This, index, wine_dbgstr_w(name));
-return E_NOTIMPL;
+
+FIXME("%p %u %s\n", This, index, wine_dbgstr_w(name));
+
It probably should be TRACE.
Re: d3d9: Partial implementation of IDirect3DSwapChain9Ex
On 8/30/2013 12:19, Michael Müller wrote:
This patch implements the IDirect3DSwapChain9Ex interface as an
extension of IDirect3DSwapChain9 and thus fixes bug 34252 - "Silverlight
accelerated graphics cause a D3D critical section lockup" since
Silverlight does no longer try to create a Direct3D context in an
endless loop. The functions d3d9_swapchain_GetLastPresentCount and
d3d9_swapchain_GetPresentStatistics are just stubs so far.
---
dlls/d3d9/swapchain.c | 71
-
include/d3d9.h| 14 +-
include/d3d9types.h |8 ++
3 files changed, 91 insertions(+), 2 deletions(-)
@@ -34,7 +34,7 @@ static HRESULT WINAPI
d3d9_swapchain_QueryInterface(IDirect3DSwapChain9 *iface,
{
TRACE("iface %p, riid %s, out %p.\n", iface, debugstr_guid(riid), out);
-if (IsEqualGUID(riid, &IID_IDirect3DSwapChain9)
+if (IsEqualGUID(riid, &IID_IDirect3DSwapChain9) || IsEqualGUID(riid,
&IID_IDirect3DSwapChain9Ex)
|| IsEqualGUID(riid, &IID_IUnknown))
{
IDirect3DSwapChain9_AddRef(iface);
@@ -221,12 +221,76 @@ static HRESULT WINAPI
d3d9_swapchain_GetPresentParameters(IDirect3DSwapChain9 *i
return D3D_OK;
}
I don't know if d3d9 is special in this regard, but this is a wrong way
to implement derived interface.
You need you make everything IDirect3DSwapChain9Ex, for example this one:
/*
- * IDirect3DSwapChain9 interface
+ * IDirect3DSwapChain9(Ex) interface
*/
#define INTERFACE IDirect3DSwapChain9
DECLARE_INTERFACE_(IDirect3DSwapChain9,IUnknown)
@@ -405,6 +405,10 @@ DECLARE_INTERFACE_(IDirect3DSwapChain9,IUnknown)
STDMETHOD(GetDisplayMode)(THIS_ D3DDISPLAYMODE* pMode) PURE;
STDMETHOD(GetDevice)(THIS_ struct IDirect3DDevice9** ppDevice) PURE;
STDMETHOD(GetPresentParameters)(THIS_ D3DPRESENT_PARAMETERS*
pPresentationParameters) PURE;
+/*** IDirect3DSwapChain9Ex methods ***/
+STDMETHOD(GetLastPresentCount)(THIS_ UINT* pLastPresentCount) PURE;
+STDMETHOD(GetPresentStats)(THIS_ D3DPRESENTSTATS* pPresentationStatistics)
PURE;
+STDMETHOD(GetDisplayModeEx)(THIS_ D3DDISPLAYMODEEX*
pMode,D3DDISPLAYROTATION* pRotation) PURE;
};
#undef INTERFACE
is no longer just IDirect3DSwapChain9. This will fix vtbl type too.
Re: [PATCH] iphlpapi: Add AllocateAndGetTcpExTableFromStack().
On 8/28/2013 13:41, Ralf Habacker wrote: Signed-off-by: Ralf Habacker --- dlls/iphlpapi/iphlpapi.spec |1 + dlls/iphlpapi/ipstats.c | 26 ++ dlls/iphlpapi/ipstats.h |1 + include/tcpmib.h|3 +++ 4 Dateien geändert, 31 Zeilen hinzugefügt(+) +DWORD WINAPI AllocateAndGetTcpExTableFromStack( PMIB_TCPTABLE_EX *ppTcpTable, BOOL bOrder, +HANDLE heap, DWORD flags ) Are you sure about this? It doesn't match a prototype from MSDN.
Re: ntdll: Store all 'comClass' attributes
On 8/26/2013 19:04, Alexandre Julliard wrote:
Nikolay Sivov writes:
+while (min <= max)
+{
+int n, c;
+
+n = (min+max)/2;
+
+c = strncmpW(olemisc_values[n].name, str, len);
strncmp isn't enough.
Right, thanks.
Re: [PATCH 1/2] advapi32/tests: Add tests for hkcr handle mask
On 8/24/2013 07:11, George Stephanos wrote: +#define IS_HKCR(hk) ((UINT_PTR)hk > 0 && ((UINT_PTR)hk & 3) == 2) + Why is that important to check? In other words what depends on particular value pattern?
Re: comsvcs: Add dll comsvcs (try 3)
On 8/23/2013 13:18, Alistair Leslie-Hughes wrote: Hi, Fix build issue +#include "rpcproxy.h" I don't think you need this. +HINSTANCE COMSVCS_hInstance = NULL; This should be static (and without initializer after that).
Re: oleaut32/tests: Some tests for LoadRegTypeLib() with activated context
On 08/22/2013 04:08 PM, Alexandre Julliard wrote: Nikolay Sivov writes: Some tests for LoadRegTypeLib() with activated context From 1ea3d8c82dd77ac14f2cdec748c0284032958b2f Mon Sep 17 00:00:00 2001 From: Nikolay Sivov Date: Wed, 21 Aug 2013 12:34:47 +0400 Subject: [PATCH 5/6] Some tests for LoadRegTypeLib() with activated context It doesn't work here: ../../../tools/runtest -q -P wine -M oleaut32.dll -T ../../.. -p oleaut32_test.exe.so tmarshal.c&& touch tmarshal.ok tmarshal.c:999: Test failed: LoadRegTypeLib failed with error 0x8002801d tmarshal.c:1232: Test failed: IKindaEnumWidget_Next failed with error 0x8007000e wine: Unhandled page fault on read access to 0x at address 0x68640714 (thread 003a), starting debugger... This was stupid. I sent a fixed version.
Re: [4/4] oledb32: A basic test for IRowPosition_Initialize()
On 8/22/2013 14:55, Marvin wrote: Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=26813 Your paranoid android. === WINEBUILD (build) === Make failed It fails to link for some reason: --- /var/lib/winetestbot/build-mingw32/dlls/oledb32/tests/../../../../wine-git/dlls/oledb32/tests/database.c:297: undefined reference to `_IID_IChapteredRowset' |--- it's added with previous patch, builds fine here. |
Re: oledb32: Add IDataSourceLocator support (try 6)
On 8/21/2013 10:33, Alistair Leslie-Hughes wrote:
+typedef struct DataSourceImpl
+{
+IDataSourceLocator IDataSourceLocator_iface;
+LONG ref;
+
+} DataSourceImpl;
All good, except this one.
Re: oledb32: Add IDataSourceLocator support (try 5)
On 8/21/2013 10:16, Alistair Leslie-Hughes wrote: Hi, Updated naming. You updated method names only, what's left : +typedef struct DataSourceImpl +HRESULT create_data_source(IUnknown *outer, void **obj) +static const IDataSourceLocatorVtbl DataSourceVtbl = Also you most likely don't need these: +#include "wine/unicode.h" +#include "wine/list.h" Changelog: oledb32: Add IDataSourceLocator support Best Regards Alistair Leslie-Hughes
Re: oledb32: Add IDataSourceLocator support (try 4)
On 8/21/2013 09:15, Alistair Leslie-Hughes wrote:
Hi,
Use helper functions for memory
Changelog:
oledb32: Add IDataSourceLocator support
Best Regards
Alistair Leslie-Hughes
It looks okay, but I suggest to fix a naming:
+typedef struct DataSourceImpl
+{
+IDataSourceLocator IDataSourceLocator_iface;
+LONG ref;
+
+} DataSourceImpl;
+static HRESULT WINAPI datasource_QueryInterface(IDataSourceLocator *iface,
REFIID riid, void **ppvoid)
etc.
IDataSource is another interface, and it's confusing to name locator
after it - dslocator_* naming used in CF is
short enough and descriptive to use it everywhere.
Re: oledb32: Implement IErrorRecord AddErrorRecord
On 8/20/2013 13:21, Alistair Leslie-Hughes wrote: Hi, Yes we do just take the pointer (pErrorInfo) passed in. MSDN states that the caller responsible for this memory. It means caller could free it at any time (or it could be a stack variable), so you have to make a deep copy. Changelog: oledb32: Implement IErrorRecord AddErrorRecord Best Regards Alistair Leslie-Hughes
Re: msxml3: Defer the conversion of XPath expressions to UTF-8. (try 4)
On 8/17/2013 21:05, John Chadwick wrote: Nikolay, does this patch look good to you? I was advised on #winehackers to get some acknowledgement that the patch is OK. Hopefully doing so can get things moving along. Yes, it looks ok. Idea is to have xmlChar closer to actual libxml2 calls that need it, instead of spreading with every top level call, and it does that.
Re: msvcrt: strncpy doesn't compliant C standard
On 8/6/2013 11:09, Álvaro Nieto wrote: This patch solves [Bug 34211]. The implementation of strncpy function doesn't compliant with C standard [1]. Also Microsoft Visual Studio C/C++ compiler is ok with the standard [2]. Extract from msdn; "The strncpy function copies the initial count characters of strSource to strDest and returns strDest. If count is less than or equal to the length of strSource, a null character is not appended automatically to the copied string. If count is greater than the length of strSource, the destination string is padded with null characters up to length count. The behavior of strncpy is undefined if the source and destination strings overlap." [1] http://pubs.opengroup.org/onlinepubs/009695399/functions/strncpy.html [2] http://msdn.microsoft.com/en-us/library/vstudio/xdsywd25.aspx This needs tests.
Re: ntdll: Implement compatible FindActCtxSectionString() for window class section (try3)
On 8/2/2013 13:33, Alexandre Julliard wrote: Nikolay Sivov writes: @@ -1031,6 +1105,17 @@ static BOOL parse_window_class_elem(xmlbuf_t* xmlbuf, struct dll_redirect* dll) if (!(entity->u.class.name = xmlstrdupW(&content))) return FALSE; +/* each class entry needs index, data and string data */ +acl->actctx->wndclass_section_len += sizeof(struct wndclass_index); +acl->actctx->wndclass_section_len += sizeof(struct wndclass_redirect_data); +/* original name is stored separately */ +acl->actctx->wndclass_section_len += aligned_string_len((content.len+1)*sizeof(WCHAR)); +/* versioned name and module name are stored one after another */ +len = get_assembly_version(assembly, NULL) + content.len + 2 /* null terminator and '!' separator */; +len += strlenW(dll->name) + 1; +acl->actctx->wndclass_section_len += aligned_string_len(len*sizeof(WCHAR)); +acl->actctx->wndclass_count++; + It would be better to do this at the time you build the structure. The parser shouldn't need to worry about this. Sure, I can do it this way. The idea was to alloc everything in one step. Would it be okay to realloc for each entry then at a time I'm about to add one?
Re: reg.exe: Add query function
On 8/2/2013 12:52, Hugh McMaster wrote: +case REG_BINARY: +case REG_NONE: +pValue = value; +for (i=0; i There's no need for separate variable here nor for incrementing pointer. The incrementing pointer is needed because the 'value' is a Byte array. But the separate variable is not needed. Yes, so value[i] will do the same.
Re: reg.exe: Add query function
On 8/1/2013 17:44, Hugh McMaster wrote:
This patch builds significantly on the query function stub in programs/reg.exe.
The patch includes recursion for subkeys.
Some examples of usage:
wine reg.exe query "HKCU"
wine reg.exe query "HKCU\Console"
wine reg.exe query "HKCU\Console" /v "ScreenSize"
wine reg.exe query "HKCU\Software" /s
---
programs/reg/reg.c | 232 +---
1 file changed, 221 insertions(+), 11 deletions(-)
+static LPVOID WINAPI mem_alloc(size_t size)
+{
+return HeapAlloc(GetProcessHeap(), 0, size);
+}
+
+static void WINAPI mem_free(LPVOID mem)
+{
+HeapFree(GetProcessHeap(), 0, mem);
+}
Why WINAPI? It's better to follow naming from existing module, there's
plenty of them with similar helpers.
+ case REG_SZ:
+case REG_MULTI_SZ:
+case REG_EXPAND_SZ:
No tabs please.
+static void reg_output_subkey(LPWSTR key_name, LPWSTR subkeyName)
+{
+reg_printfW(formatSW, key_name);
+
+if (subkeyName > 0)
+{
+reg_printfW(formatSW, slashW);
+reg_printfW(formatSW, subkeyName);
+reg_printfW(formatSW, newlineW);
+}
+}
Comparison looks a bit odd. Also why do you need to break it down to
three printf calls instead of one?
+p = strchrW(key_name,'\\');
+if (!p)
+{
+p = 0;
+}
+else p++;
I'm not sure what this is supposed to do.
+case REG_BINARY:
+case REG_NONE:
+pValue = value;
+for (i=0; i
There's no need for separate variable here nor for incrementing pointer.
Re: NtCreateFile fails
On 7/29/2013 18:02, Mislav Blazevic wrote:
First of all, pardon my ignorance, I am new to wine and winapi.
I am trying to implement apphelp.dll and I ran into issues with
NtCreateFile: It always returns 0xc103, STATUS_NOT_A_DIRECTORY,
and I am not sure what that means. Here is code:
/* This function is not in API, but written for internal use */
HANDLE WINAPI SdbOpenFile( LPCWSTR path, PATH_TYPE type )
{
/* SNIP */
if (type == DOS_PATH)
if (!RtlDosPathNameToNtPathName_U(path, &str, NULL, NULL))
return NULL;
InitializeObjectAttributes(&attr, &str, OBJ_CASE_INSENSITIVE, NULL, NULL);
status = NtCreateFile(&file, GENERIC_READ | SYNCHRONIZE |
FILE_READ_ATTRIBUTES,
&attr, &io, NULL, FILE_ATTRIBUTE_NORMAL, FILE_SHARE_READ,
FILE_OPEN, FILE_DIRECTORY_FILE, NULL, 0);
/* SNIP */
return file;
}
Function is called like this:
WCHAR szFileName[1024] = {0};
ExpandEnvironmentStringsW(L"%SystemRoot%\\sample.sdb", szFileName,
1024); // for testing purposes
SdbOpenFile(szFileName, DOS_PATH);
Compiled with winegcc.
I am sure that C:\windows\sample.sdb exists. What am I doing wrong?
Probably you don't need FILE_DIRECTORY_FILE if it's not a directory?
Re: gdi32: Don't let the gdi32 driver allocate memory in the dplayx region.
On 7/26/2013 08:48, Alexei Svitkine wrote: On Mon, Jul 22, 2013 at 10:45 PM, Alexei Svitkine mailto:alexei.svitk...@gmail.com>> wrote: Don't let the gdi32 driver allocate memory in the dplayx region. Fixes bug: http://bugs.winehq.org/show_bug.cgi?id=34095 The issue is that the dplayx code in wine needs to allocate some memory at a static address (0x5000) and loading the Mac native gdi driver ends up using that part of the address space. This patch fixes that by reserving that part of memory while the display driver is initialized and releasing it after. --- dlls/gdi32/driver.c | 10 ++ 1 file changed, 10 insertions(+) Please stops resending this. Looks like dplay should be fixed to not depend on that reserved address area.
Re: [PATCH 1/6] scrrun: Store reference count inside IFileSystem3 object
On 7/25/2013 16:35, Piotr Caban wrote: --- dlls/scrrun/filesystem.c | 163 +++ 1 file changed, 123 insertions(+), 40 deletions(-) Why do you need this?
Re: [1/5] oledb32: Stub for IRowPosition
On 7/23/2013 12:55, Marvin wrote: Hi, While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at http://testbot.winehq.org/JobDetails.pl?Key=26406 Your paranoid android. === WINEBUILD (build) === Make failed I don't really know why it fails here, probably something to do with idls or something else.
Re: [PATCH 2/2] include/ddk: add usbioctl.h
On 7/22/2013 22:38, Damjan Jovanovic wrote: Changelog: * add usbioctl.h Damjan Jovanovic Hi, Damjan. You forgot patches.
Re: msxml3: Defer the conversion of XPath expressions to UTF-8. (try 2)
On 7/20/2013 23:37, John Chadwick wrote:
This patch changes the create_selection interface to accept a wide string
instead of an xmlChar string. This is a transitional change, which will be
needed as libxml2's XPath parser is phased out.
The last submission had a memory leak (sorry, I isolated this change
from some other changes and accidentally axed a heap_free in the
process.)
Looks ok, I didn't try tests, but I hope you did..
One thing though:
+static inline WCHAR *heap_strcatW(WCHAR *a, const WCHAR *b)
+{
+int len = strlenW(a) + strlenW(b);
+WCHAR *c = heap_realloc(a, (len + 1) * sizeof(WCHAR));
+WCHAR *ptr = c;
+
+while(*ptr)
+ptr++;
+while(*b)
+*(ptr++) = *(b++);
+c[len] = 0;
+
+return c;
+}
+
I'm assuming you didn't use strcatW() to avoid one more strlenW()? Even
if you don't want to use
it for some reason, you don't need first loop cause you already know 'a'
length, and second loop
is just strcpyW(). Also I suggest to use 'dst', 'src' names for parameters.
Re: riched20: Add tests for ITextRange interface.
On 7/19/2013 00:15, Caibin Chen wrote: What files do you mean to merge? txtsrv.c and richole.c or test/richole.c and test/richole.c? Tests of course. Another solution is to create ITextService instance in richole.c and run range tests or whatever tests you need on it. It's a couple of lines to add. 2013/7/18 Nikolay Sivov : On 7/19/2013 00:04, Caibin Chen wrote: The ITextRange objects are returned by ITextDocument_Range() method. Both IRichEditOle and ITextService supports ITextDocument interface. Currently we don't have ITextDocument support in ITextService. my another patch[1] is to add ITextDocument interface support in ITextService. I see, then maybe it's better to merge two existing files first. [1]. http://www.winehq.org/pipermail/wine-patches/2013-July/125258.html 2013/7/18 Nikolay Sivov : On 7/18/2013 23:36, Caibin Chen wrote: ITextRange will be used in both richole.c and txtsrv.c. I'm planning to add more tests in it. I think it makes more sense to have one test file for each interface, which improves the readability of tests. That's usually done for implementation, but not for the tests. What does it mean it will be used in both files by the way? 2013/7/18 Nikolay Sivov : On 7/18/2013 20:28, Caibin Chen wrote: Add tests for start and end point, setting and getting text. --- dlls/riched20/tests/Makefile.in | 3 +- dlls/riched20/tests/txtrng.c| 343 2 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 dlls/riched20/tests/txtrng.c You could just use existing file, richole.c fits I think.
Re: riched20: Add tests for ITextRange interface.
On 7/19/2013 00:04, Caibin Chen wrote: The ITextRange objects are returned by ITextDocument_Range() method. Both IRichEditOle and ITextService supports ITextDocument interface. Currently we don't have ITextDocument support in ITextService. my another patch[1] is to add ITextDocument interface support in ITextService. I see, then maybe it's better to merge two existing files first. [1]. http://www.winehq.org/pipermail/wine-patches/2013-July/125258.html 2013/7/18 Nikolay Sivov : On 7/18/2013 23:36, Caibin Chen wrote: ITextRange will be used in both richole.c and txtsrv.c. I'm planning to add more tests in it. I think it makes more sense to have one test file for each interface, which improves the readability of tests. That's usually done for implementation, but not for the tests. What does it mean it will be used in both files by the way? 2013/7/18 Nikolay Sivov : On 7/18/2013 20:28, Caibin Chen wrote: Add tests for start and end point, setting and getting text. --- dlls/riched20/tests/Makefile.in | 3 +- dlls/riched20/tests/txtrng.c| 343 2 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 dlls/riched20/tests/txtrng.c You could just use existing file, richole.c fits I think.
Re: riched20: Add tests for ITextRange interface.
On 7/18/2013 23:36, Caibin Chen wrote: ITextRange will be used in both richole.c and txtsrv.c. I'm planning to add more tests in it. I think it makes more sense to have one test file for each interface, which improves the readability of tests. That's usually done for implementation, but not for the tests. What does it mean it will be used in both files by the way? 2013/7/18 Nikolay Sivov : On 7/18/2013 20:28, Caibin Chen wrote: Add tests for start and end point, setting and getting text. --- dlls/riched20/tests/Makefile.in | 3 +- dlls/riched20/tests/txtrng.c| 343 2 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 dlls/riched20/tests/txtrng.c You could just use existing file, richole.c fits I think.
Re: riched20: Add tests for ITextRange interface.
On 7/18/2013 20:28, Caibin Chen wrote: Add tests for start and end point, setting and getting text. --- dlls/riched20/tests/Makefile.in | 3 +- dlls/riched20/tests/txtrng.c| 343 2 files changed, 345 insertions(+), 1 deletion(-) create mode 100644 dlls/riched20/tests/txtrng.c You could just use existing file, richole.c fits I think.
Re: ntdll: Tests for RtlHashUnicodeString()
On 7/16/2013 09:47, Dmitry Timoshkov wrote: Nikolay Sivov wrote: +#include "ddk/wdm.h" Please don't do that, this breaks compilation with PSDK headers. If you need some specific definitions, types or API prototypes - replicate them in the test itself. What breaks exactly? The fact that PSDK doesn't have DDK headers by default or something specific about this particular header? If it's a missing header I don't see a problem with installing DDK. Also kernel32 tests have include from ddk subdir too and I don't even have this header installed with DDK 7.something, is it broken for you too?
Re: [PATCH] imm32: Fixed crashing in ImmGetIMCCSize.
On 7/9/2013 18:52, Qian Hong wrote: Hi Nikolay, Thanks for comments! On Tue, Jul 9, 2013 at 9:01 PM, Nikolay Sivov wrote: Do you really need this for Office 2010 or null check is enough? If it's passing 0 handle here that's what you should check imho. If HIMCC is really a handle and not a struct pointer like in wine, that's not how a handle validity check will look like. null check is not enough, see the below quoted log: --- snip --- 0029:Call imm32.ImmGetIMCCSize(bf9c73e5) ret=0d6d3545 --- snip --- This could be some a different bug, and putting exception handler around it is not necessary a right solution. Another example: --- snip --- 0029:Call imm32.ImmGetIMCCSize(0190) ret=0a193545 --- snip --- Could you provide more details for how to check the validity of the handle in the right way? Well, if it's really supposed to be a handle, meaning it's a table index (with some offset or not), this check will simply check for index boundaries and whether slot is allocated. This all need some tests. Thanks a lot! -- Regards, Qian Hong - http://www.winehq.org
Re: [PATCH] imm32: Fixed crashing in ImmGetIMCCSize.
On 7/9/2013 16:50, Qian Hong wrote:
Fixed a crash in MS Office 2010 Chinese version.
Thanks Jiang Yike for the helps on testing :)
---
dlls/imm32/imm.c |6 ++
dlls/imm32/tests/imm32.c | 13 +
2 files changed, 19 insertions(+)
+if (IsBadReadPtr(internal, sizeof(HIMCC)))
+{
+SetLastError(ERROR_INVALID_HANDLE);
+return 0;
+}
+
Do you really need this for Office 2010 or null check is enough? If it's
passing 0 handle here
that's what you should check imho. If HIMCC is really a handle and not a struct
pointer like in wine,
that's not how a handle validity check will look like.
Re: msxml3/tests: add additional conformance tests that show incorrect xpath behavior in wine
On 6/29/2013 20:26, John Chadwick wrote:
On Sat, Jun 29, 2013 at 4:48 AM, Nikolay Sivov wrote:
On 6/27/2013 11:46, John Chadwick wrote:
+/* msxml's selectNodes returns a document ordered nodelist,
regardless of
+ * whether or not the xpath nodelist was document ordered... */
+
ole_check(IXMLDOMNode_selectNodes(rootNode,_bstr_("ancestor-or-self::node()"),
&list));
+todo_expect_list_and_release(list, "D1 E2.D1");
Please use explicit check for return value, like ok(hr == S_OK, "...");,
Which return value am I missing? FWIW, I'm doing what the other tests
are doing, as far as I can tell.
What I mean is that use of macros like ole_check() is discouraged, it's
better to have explicit check for return value.
also you probably don't need another macro for list checking,
isn't it enough to wrap expect_list_and_release() in todo_wine {} ?
Could it be that simple? I tried to do
todo_wine expect_list_and_release(list, "D1 E2.D1");
But it fails before the actual test occurs, because another test
inside the macro succeeds, apparently. Are you saying if I wrap the
expect_list_and_release macro call in a scope, it will work? I will
try.
Yes, I see what you mean. In order to improve situation I'd suggest to
move to using
table of test queries and results. This way you have less code and no
duplication,
todo flag should just be stored in test data. We already have some tests
done this way,
it's much easier to maintain.
I figured creating a new macro would be a better solution than
duplicating the code separately in two locations, but if it'd be
better to just not use the macro, then that's simple enough to fix.
Re: po: Update Czech translation - 2nd attempt
On 6/28/2013 18:21, Pavel Borecki wrote: Hi, Pavel. Looks like you're using poedit or something with similar habit of touching lines location headers: --- -#: appwiz.rc:66 appwiz.rc:42 cryptui.rc:343 msacm32.rc:37 winecfg.rc:191 -#: winecfg.rc:228 wordpad.rc:245 +#: appwiz.rc:66 +#: appwiz.rc:42 +#: cryptui.rc:343 +#: msacm32.rc:37 +#: winecfg.rc:191 +#: winecfg.rc:228 +#: wordpad.rc:245 msgid "&Remove" msgstr "Odeb&rat" --- This adds a lot of diff lines that you don't really want.
Re: msxml3/tests: add additional conformance tests that show incorrect xpath behavior in wine
On 6/27/2013 11:46, John Chadwick wrote:
+/* msxml's selectNodes returns a document ordered nodelist, regardless of
+ * whether or not the xpath nodelist was document ordered... */
+ole_check(IXMLDOMNode_selectNodes(rootNode,_bstr_("ancestor-or-self::node()"),
&list));
+todo_expect_list_and_release(list, "D1 E2.D1");
Please use explicit check for return value, like ok(hr == S_OK, "...");,
also you probably don't need another macro for list checking,
isn't it enough to wrap expect_list_and_release() in todo_wine {} ?
Re: po: Update Ukrainian translation
On 6/26/2013 13:10, Микола wrote: Hi, please resubmit this patch with your full name transliterated in English.
Re: oledb32: Add IErrorInfo Support
On 6/14/2013 14:12, Alistair Leslie-Hughes wrote:
+typedef struct ErrorInfoImpl
+{
+IErrorInfo IErrorInfo_iface;
+LONG ref;
+
+GUID m_Guid;
+WCHAR *source;
+WCHAR *description;
+WCHAR *help_file;
+DWORD m_dwHelpContext;
+} ErrorInfoImpl;
Please use consistent naming, for example avoid prefixed names.
Also it would be cleaner to use BSTR here, but it will work either way.
+TRACE("(%p)->(pBstrSource=%p)\n",This,pBstrSource);
No need for parameter names in traces.
Re: oledb32: Correct DBPROP_INIT_PROVIDERSTRING and DBPROP_INIT_HWND types
On Thu, Jun 6, 2013 at 9:55 AM, Alistair Leslie-Hughes <
leslie_alist...@hotmail.com> wrote:
> Hi,
> Corrects a spelling mistake too and
>
> Changelog:
> oledb32: Correct DBPROP_INIT_PROVIDERSTRING and DBPROP_INIT_HWND types
>
>
> Best Regards
> Alistair Leslie-Hughes
>
>
>
>
-{ extemdedW, DBPROP_INIT_PROVIDERSTRING,
DBPROPOPTIONS_REQUIRED, VT_BSTR },
+{ extemdedW, DBPROP_INIT_PROVIDERSTRING,
DBPROPOPTIONS_OPTIONAL, VT_BSTR },
{ gen_timeout, DBPROP_INIT_GENERALTIMEOUT,
DBPROPOPTIONS_OPTIONAL, VT_I4 },
-{ initcatW,DBPROP_CATALOGLOCATION,
DBPROPOPTIONS_OPTIONAL, VT_BSTR },
+{ initcatW,DBPROP_CATALOGLOCATION,
DBPROPOPTIONS_OPTIONAL, VT_I4 },
How it's an integer? Catalog location is a path basically. Regarding
provider string, could you please add a test for this flag?
Re: [1/2] DWrite: Implemented functions regarding text alignment. (try 3)
On Fri, May 31, 2013 at 9:07 PM, Fabian Müntefering wrote: > Implemented functions of text format and text layout regarding text > alignment. > > Try 3: Used proper enum value to set the enum variables. > > > > > +if(alignment!=DWRITE_TEXT_ALIGNMENT_CENTER && alignment!=DWRITE_TEXT_ALIGNMENT_LEADING && + alignment!=DWRITE_TEXT_ALIGNMENT_JUSTIFIED && alignment!=DWRITE_TEXT_ALIGNMENT_TRAILING) +return E_INVALIDARG; +This->format.text_align = alignment; This doesn't look great, you'll need a test that shows it really rejects meaningless values, and if it does it would be simpler to compare with greatest enum value.
Re: [1/3] DWrite: Added missing JUSTIFIED text alignment mode.
Hi, Fabian. You forgot to attach patches 2 and 3.
Re: [5/5] oledb32: GetConversionSize only returns a valid size of BSTR
On Thu, May 23, 2013 at 11:48 AM, Alistair Leslie-Hughes < leslie_alist...@hotmail.com> wrote: > Hi, > > > Changelog: > oledb32: GetConversionSize only returns a valid size of BSTR > +if(V_VT((VARIANT*)src) != VT_BSTR) +*dst_len = 110; else -return hr; +*dst_len = (SysStringLen(V_BSTR((VARIANT*)src)) + 1) * sizeof(WCHAR); + +return S_OK; So it returns previous result if type is invalid, meaning it's not VT_BSTR? You can't just return a random constant that appears in tests just because of previously made call (which is my guess). If you really want to implement this broken behaviour it will need more tests, at least one more that shows it returns a result from last successful call. > > Best Regards > Alistair Leslie-Hughes > > > >
Re: riched20: Added tests for ITextDocument::Open
On Tue, May 21, 2013 at 3:45 PM, Jactry Zeng wrote: > > > > > If the whole purpose of storing flags names is to output readable debug info I think it's better to have a single separate function that takes flags combination and returns static buffer pointer with a name. This way it will need just to append after bitwise 'and' check. + /* tests for ITextDocument::Open */ + for(; tomNunSingle >= 0; tomNunSingle--) Do you really need to go from the tail here?
Re: [1/4] xolehlp: fix calling convention
On 5/1/2013 11:53, Daniel Jeliński wrote: Ack, done. By the way, I'm looking for txdtc.idl (SQL Server wants IResourceManagerFactory, which is declared there), but Google only finds txdtc.h. Do we have any tool to generate IDL back from header? I don't think so, it's kind of normal for PSDK to include generated header only for some files. Not sure why they do that, probably to have them ready to include and skip generation step for end user. Also it's cleaner to write it by hand in terms of copyrighted materials, that's mean you can't copy parts or whole file from PSDK/VS installation but you're free to look at definitions. Thanks, Daniel 2013/5/1 Nikolay Sivov <mailto:bungleh...@gmail.com>> On 5/1/2013 01:43, Daniel Jeliński wrote: Please fix spec file accordingly.
Re: [1/4] xolehlp: fix calling convention
On 5/1/2013 01:43, Daniel Jeliński wrote: Please fix spec file accordingly.
Re: riched20: Fix richtext copy/paste to OOffice.
On 4/30/2013 10:20, Sergey Guralnik wrote: When copying cyrillic text from wordpad (or any richedit) to Open Office document I get unreadable text. It seems like some another codepage was used to display text. This path solves this problem. --- dlls/riched20/writer.c |8 1 files changed, 8 insertions(+), 0 deletions(-) Is it really supposed to do that? Does native module help?
Re: [PATCH 1/2] shell32: Add a semi-stub for SHGetStockIconInfo
On Wed, Apr 24, 2013 at 6:38 PM, Detlef Riekenberg wrote:
> --
> By by ... Detlef
> ---
> dlls/shell32/iconcache.c | 42
> ++
> dlls/shell32/shell32.spec |1 +
> 2 files changed, 43 insertions(+), 0 deletions(-)
>
> diff --git a/dlls/shell32/iconcache.c b/dlls/shell32/iconcache.c
> index 637dbca..df6675d 100644
> --- a/dlls/shell32/iconcache.c
> +++ b/dlls/shell32/iconcache.c
> @@ -46,6 +46,8 @@
>
> WINE_DEFAULT_DEBUG_CHANNEL(shell);
>
> +static const WCHAR shell32dll[] =
> {'\\','s','h','e','l','l','3','2','.','d','l','l',0};
> +
>
Please put this in function that needs it.
Re: oledb32: Implement GetConversionSize DBTYPE_WSTR->DBTYPE_WSTR
On 4/20/2013 13:43, Alistair Leslie-Hughes wrote: Hi, Changelog: oledb32: Implement GetConversionSize DBTYPE_WSTR->DBTYPE_WSTR Best Regards Alistair Leslie-Hughes Please add a test when caller supplies source length, at least one case is interesting - supplied length is less than lstrlenW result. Another case would be embedded nulls and source length larger than null terminated length.
Re: zlib's gzseek return gabbage and fails intermittently under wine.
On 4/15/2013 02:50, Hin-Tak Leung wrote:
--- On Sun, 14/4/13, Vincent Povirk wrote:
Well, here's a simple thing you can
check: Does your zlib dll link to
_lseek or _lseeki64? The first one uses a 32-bit offset.
Wine's
implementation (http://source.winehq.org/source/dlls/msvcrt/file.c#L1090)
expands that to 64-bit and later truncates the file offset
to 32-bit.
For a file larger than 2 GB, that could account for the
large negative
value you're seeing.
And since this would only matter in cases where zlib uses
lseek (the
first time through I guess it wouldn't, as it has to read
the whole
everything up to the offset you give at least once) and is
at least 2
GB into the file, that might also explain why it doesn't
fail
initially.
But without really digging into the zlib code, all I can do
it speculate.
I should probably also check coapp's build of zlib
sometime.
It is not a dll - as you suggested and I already wrote, due to past experience
of other's packaging of slightly outdated, it is being built against a private
*source* copy of the latest zlib.
Also the bogus offset is not large negative but large (larger than 2^32)
positive.
Here is an example of the debug output under wine:
---
set_filepos failed at 34307 returning 134127533721091
Re-opening to re-try
Retry successful
set_filepos failed at 96919 returning 146686018157207
Re-opening to re-try
Retry successful
set_filepos failed at 128254 returning 12103217968382
Re-opening to re-try
Retry successful
...
---
This is generated by this code snipplet which is called inside a loop, all
wrapped in the c++ class:
---
off_t offset = gzseek(gzvcf_in, filepos, SEEK_SET);
if (offset != filepos) { //implicitly converted to off_t by
template streamoff()
LOG.printLOG("set_filepos failed at " +
LOG.streampos2str(filepos)
+ " returning " + LOG.off_t2str(offset) + "\n");
LOG.printLOG("Re-opening to re-try\n");
close(); open();
off_t offset1 = gzseek(gzvcf_in, filepos, SEEK_SET);
if (offset1 == filepos)
LOG.printLOG("Retry successful\n");
else
LOG.error("Retry failed\n"); // this also aborts
}
---
This code runs silently on linux i.e. the "if (offset != filepos)" condition is
not triggered.
For windows build you'll need to define _WIN32, so _lseeki64 is used by
zlib. After this done you could play with
native msvcrt to see if it helps, and after that +relay will tell you
everything.
Re: GSoC 2013 proposal: Implement XPath from scratch
On 4/11/2013 05:52, John Chadwick wrote: Hello, I am a student of computer science interested in entering Wine development via GSoC 2013. Upon looking at the various suggested proposals, the project of reimplementing XPath without libxml2 looked particularly interesting. I am familiar with the C language, W3C DOM and XPath, and I believe this is something I could do. I have a few questions I'd be interested in seeing answered: * One of the 'important things to keep in mind' is that "changes should be incremental, with a fallback to libxml2 until new processor is fully functional." Does this mean at compile time, or at runtime, when the evaluator recognizes unimplemented features in an expression, or something else entirely? Yes, what I meant is that a transition should be as smooth as possible, it should be a runtime check like you described - if processor doesn't support something that's needed to process given expression it shouldn't fail completely, but fallback to existing code. * Is only MSXML 3.0 supported? It appears to be the only version implemented in the Wine source tree. It's just a way it's done. All code is in msxml3 dir, and msxml4 and msxml6 just forward to it, wine's msxml3 is able to create coclass versions that normally would live in msxml4 or msxml6. That's how code duplication is resolved. * What should I aim at doing first? Because what I'd be working on is a library, I would probably need some application to test features with (test cases aside.) Should I be using an existing program, or writing a new one? You should be writing tests for XPath API, and it should be placed somewhere in msxml3/tests. Currently it lives in domdoc.c cause XPath is a part of DOM API, but it will be probably okay to create a separate file if amount of tests is expected to be large enough (and it is imho). Speaking of tests, it should be easily extendible meaning that adding more tests should be mostly a matter of adding more test data and not that much of a code. Currently in existing XPath tests returned node set forms some string representation and we compare these strings with expected ones. I don't really like this way mostly cause it's hard to read and relate to original tree, but we could discuss that and make it better or find another way. By the way, the main reason why I added this topic to GSoC page was that it seems to me that it's possible to implement it incrementally, this is a very important, and XPath implementation is a significant part of a long term plan to stop using libxml2 and have our own code (likely a lot of own code). If anyone has anything to add here or is even opposed to such project for some reason, I'd really like to hear that. Thanks, John Chadwick
Re: [3/13] wineconsole and kernel32: set GetLargestConsoleWindowSize based on screen resolution
On Mon, Apr 8, 2013 at 4:47 PM, Hugh McMaster < hugh.mcmas...@masterindexing.com> wrote: > From: Nikolay Sivov [mailto:bungleh...@gmail.com] > Sent: Monday, 8 April 2013 11:08 PM > To: wine-devel; Hugh McMaster > Subject: Re: [3/13] wineconsole and kernel32: set > GetLargestConsoleWindowSize based on screen resolution > > >On Mon, Apr 8, 2013 at 3:53 PM, Hugh McMaster < > hugh.mcmas...@masterindexing.com> wrote: > >This file recreates most, if not all, of the source found in > dlls/advapi32/registry.c in dlls/kernel32. It provides registry > functionality. > > > >>Why do you need this? > If I remember correctly, there were several function dependencies for the > registry implementation I worked on. I'm not sure of the exact number, but > will work this out. > > What should I do then, though? Should I isolate the functions I need into > a separate file? Or should the dlls/advapi32/registry.c be linked into the > dlls/kernel32/Makefile.in? > If you need to access registry from kernel32 you'll need to use ntdll calls directly. > > >>(not to mention that you can't just duplicate it like that) > I'm not sure what you mean. The source remains correctly attributed as in > the original. > This functionality belongs to advapi32. Do you really need anything more than ntdll calls provide?
Re: [3/13] wineconsole and kernel32: set GetLargestConsoleWindowSize based on screen resolution
On Mon, Apr 8, 2013 at 3:53 PM, Hugh McMaster < hugh.mcmas...@masterindexing.com> wrote: > This file recreates most, if not all, of the source found in > dlls/advapi32/registry.c in dlls/kernel32. It provides registry > functionality. > > Tested on Linux Mint 14 and Mac OS X 10.8 (Mountain Lion). > > --- > dlls/kernel32/advapi32_registry.c | 2928 > + > 1 file changed, 2928 insertions(+) > create mode 100644 dlls/kernel32/advapi32_registry.c > > > Why do you need this? (not to mention that you can't just duplicate it like that)
Re: [1/3] include: Add COM interface definitions needed for PrintDlgEx implementation.
Why not use DECLARE_INTERFACE_ here? And why bother checking UNICODE?
On Mon, Apr 8, 2013 at 12:10 PM, Dmitry Timoshkov wrote:
> ---
> dlls/uuid/uuid.c | 1 +
> include/commdlg.h | 80
> +++
> 2 files changed, 81 insertions(+)
>
> diff --git a/dlls/uuid/uuid.c b/dlls/uuid/uuid.c
> index fd96fe3..fb06f2d 100644
> --- a/dlls/uuid/uuid.c
> +++ b/dlls/uuid/uuid.c
> @@ -83,6 +83,7 @@ DEFINE_GUID(GUID_NULL,0,0,0,0,0,0,0,0,0,0,0);
> #include "sensevts.h"
> #include "ocmm.h"
> #include "commoncontrols.h"
> +#include "commdlg.h"
> #include "tlogstg.h"
> #include "msdasc.h"
>
> diff --git a/include/commdlg.h b/include/commdlg.h
> index 1a6e1d9..6eeb615 100644
> --- a/include/commdlg.h
> +++ b/include/commdlg.h
> @@ -762,6 +762,86 @@ typedef struct tagPDEXW
> DECL_WINELIB_TYPE_AW(PRINTDLGEX)
> DECL_WINELIB_TYPE_AW(LPPRINTDLGEX)
>
> +#ifdef STDMETHOD
> +DEFINE_GUID(IID_IPrintDialogCallback,
> 0x5852a2c3,0x6530,0x11d1,0xb6,0xa3,0x00,0x00,0xf8,0x75,0x7b,0xf9);
> +typedef interface IPrintDialogCallback IPrintDialogCallback;
> +
> +#if defined(__cplusplus) && !defined(CINTERFACE)
> +MIDL_INTERFACE("5852a2c3-6530-11d1-b6a3-f8757bf9")
> +IPrintDialogCallback : public IUnknown
> +{
> +virtual HRESULT STDMETHODCALLTYPE InitDone() = 0;
> +virtual HRESULT STDMETHODCALLTYPE SelectionChange() = 0;
> +virtual HRESULT STDMETHODCALLTYPE
> HandleMessage(HWND,UINT,WPARAM,LPARAM,LRESULT *) = 0;
> +};
> +#else
> +typedef struct IPrintDialogCallbackVtbl
> +{
> +BEGIN_INTERFACE
> +
> +/*** IUnknown methods ***/
> +HRESULT (STDMETHODCALLTYPE *QueryInterface)(IPrintDialogCallback
> *This,REFIID riid,void **ppvObject);
> +ULONG (STDMETHODCALLTYPE *AddRef)(IPrintDialogCallback *This);
> +ULONG (STDMETHODCALLTYPE *Release)(IPrintDialogCallback *This);
> +/*** IPrintDialogCallback methods ***/
> +HRESULT (STDMETHODCALLTYPE *InitDone)(IPrintDialogCallback *This);
> +HRESULT (STDMETHODCALLTYPE *SelectionChange)(IPrintDialogCallback
> *This);
> +HRESULT (STDMETHODCALLTYPE *HandleMessage)(IPrintDialogCallback
> *,HWND,UINT,WPARAM,LPARAM,LRESULT *);
> +
> +END_INTERFACE
> +} IPrintDialogCallbackVtbl;
> +interface IPrintDialogCallback
> +{
> +CONST_VTBL IPrintDialogCallbackVtbl *lpVtbl;
> +};
> +#endif /* CINTERFACE */
> +
> +DEFINE_GUID(IID_IPrintDialogServices,
> 0x509aaeda,0x5639,0x11d1,0xb6,0xa1,0x00,0x00,0xf8,0x75,0x7b,0xf9);
> +typedef interface IPrintDialogServices IPrintDialogServices;
> +
> +#if defined(__cplusplus) && !defined(CINTERFACE)
> +MIDL_INTERFACE("509aaeda-5639-11d1-b6a1-f8757bf9")
> +IPrintDialogServices : public IUnknown
> +{
> +#ifdef UNICODE
> +virtual HRESULT STDMETHODCALLTYPE GetCurrentDevMode(LPDEVMODEW,UINT
> *) = 0;
> +virtual HRESULT STDMETHODCALLTYPE GetCurrentPrinterName(LPWSTR,UINT
> *) = 0;
> +virtual HRESULT STDMETHODCALLTYPE GetCurrentPortName(LPWSTR,UINT *) =
> 0;
> +#else
> +virtual HRESULT STDMETHODCALLTYPE GetCurrentDevMode(LPDEVMODEA,UINT
> *) = 0;
> +virtual HRESULT STDMETHODCALLTYPE GetCurrentPrinterName(LPSTR,UINT *)
> = 0;
> +virtual HRESULT STDMETHODCALLTYPE GetCurrentPortName(LPSTR,UINT *) =
> 0;
> +#endif
> +};
> +#else
> +typedef struct IPrintDialogServicesVtbl
> +{
> +BEGIN_INTERFACE
> +
> +/*** IUnknown methods ***/
> +HRESULT (STDMETHODCALLTYPE *QueryInterface)(IPrintDialogServices
> *This,REFIID riid,void **ppvObject);
> +ULONG (STDMETHODCALLTYPE *AddRef)(IPrintDialogServices *This);
> +ULONG (STDMETHODCALLTYPE *Release)(IPrintDialogServices *This);
> +/*** IPrintDialogServices methods ***/
> +#ifdef UNICODE
> +HRESULT (STDMETHODCALLTYPE *GetCurrentDevMode)(IPrintDialogServices
> *,LPDEVMODEW *,UINT *);
> +HRESULT (STDMETHODCALLTYPE
> *GetCurrentPrinterName)(IPrintDialogServices *,LPWSTR,UINT *);
> +HRESULT (STDMETHODCALLTYPE *GetCurrentPortName)(IPrintDialogServices
> *,LPWSTR,UINT *);
> +#else
> +HRESULT (STDMETHODCALLTYPE *GetCurrentDevMode)(IPrintDialogServices
> *,LPDEVMODEA *,UINT *);
> +HRESULT (STDMETHODCALLTYPE
> *GetCurrentPrinterName)(IPrintDialogServices *,LPSTR,UINT *);
> +HRESULT (STDMETHODCALLTYPE *GetCurrentPortName)(IPrintDialogServices
> *,LPSTR,UINT *);
> +#endif
> +
> +END_INTERFACE
> +} IPrintDialogServicesVtbl;
> +interface IPrintDialogServices
> +{
> +CONST_VTBL IPrintDialogServicesVtbl *lpVtbl;
> +};
> +#endif /* CINTERFACE */
> +#endif /* STDMETHOD */
> +
> BOOL WINAPI ChooseColorA(LPCHOOSECOLORA lpChCol);
> BOOL WINAPI ChooseColorW(LPCHOOSECOLORW lpChCol);
> #define ChooseColor WINELIB_NAME_AW(ChooseColor)
> --
> 1.8.2
>
>
>
>
Re: wbemprox: Implement some properties of Win32_ComputerSystem and Win32_DiskPartition.
Please use your real name. On Sun, Apr 7, 2013 at 7:46 PM, mog422 wrote: > > > > >
Re: [PATCH] winspool.drv: Check the value of pDeviceName.(try 2)
On Wed, Apr 3, 2013 at 1:31 PM, Tatyana Fokina wrote: > Function DocumentPropertiesA calling GDI_CallExtDeviceMode16: If value > pDeviceName is "", from GDI_CallExtDeviceMode16 return -1. > > Is it possible to add a test for that?
Re: comctl32: Make an attempt to update notification handle
On 3/27/2013 00:10, Daniel Jeliński wrote: 2013/3/26 Nikolay Sivov : Is there any reliable way to test send mouse messages? Cause it looks like if I send WM_LBUTTONDOWN directly it doesn't trigger tab selection code on windows, but works in wine. Do you want to figure out the correct sequence of messages or write tests for wine's test suite? I need a test that actually works. Either way, it's a good idea to start with winetricks comctl32. After that WINEDEBUG=+message gives you all information you may need. That's a really questionable thing to do cause you'll be looking at native module internals, tracing what it does. I think it should be avoided. Also some message handlers use mouse coordinates from lParam, while others seem to use either GetCursorPos or GetMessagePos, so setting cursor position may be necessary. I didn't think about actually positioning cursor, that could help, thanks. Regards, Daniel
Re: iads.idl: Add missing interfaces
On 3/27/2013 09:05, Alistair Leslie-Hughes wrote:
Hi,
Changelog:
iads.idl: Add missing interfaces
Best Regards
Alistair Leslie-Hughes
+typedef enum
+{
+ADS_SEARCHPREF_ASYNCHRONOUS = 0,
+ADS_SEARCHPREF_DEREF_ALIASES= 1,
You don't need to do it manually, widl will generate values for each entry.
Re: comctl32: Make an attempt to update notification handle
On 3/25/2013 07:48, Dmitry Timoshkov wrote: Why not just call GetParent() every time when a notification is being sent? Because it won't work if you create a control without WS_CHILD. How is that different from the WM_WINDOWPOSCHANGING handler? How what is different? There's a check for null parent as you can see. Why the same can't be done at the time a notification needs to be sent? There are many places (starting from user32/WM_PARENTNOTIFY) which send notifications that way, what's so special about tab control? Unfortunately it's more complicated than that. For example notifications sent after TCM_SETCURFOCUS are delivered to original window, but if you do any mouse interaction it appears to notify new parent. So SetParent() leaves it in a kind of broken state. So we can't always use parent window, but for some notification cases valid GetParent() value should be used as in examples you mentioned. Is there any reliable way to test send mouse messages? Cause it looks like if I send WM_LBUTTONDOWN directly it doesn't trigger tab selection code on windows, but works in wine. Thanks for review.
Re: comctl32: Make an attempt to update notification handle
On 3/25/2013 07:34, Dmitry Timoshkov wrote:
Nikolay Sivov wrote:
+case WM_WINDOWPOSCHANGING:
+{
+ HWND parent = GetParent(hwnd);
+ if (parent) infoPtr->hwndNotify = parent;
+ return DefWindowProcW(hwnd, uMsg, wParam, lParam);
+}
What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING
to the original proc?
Then it will fail just like it does currently, and will work for the
rest of the applications.
Is it really supposed to fail?
That's a different problem.
Why not just call GetParent() every time when a
notification is being sent?
Because it won't work if you create a control without WS_CHILD.
How is that different from the WM_WINDOWPOSCHANGING handler?
How what is different? There's a check for null parent as you can see.
Re: comctl32: Make an attempt to update notification handle
On 3/25/2013 07:24, Dmitry Timoshkov wrote:
Nikolay Sivov wrote:
+case WM_WINDOWPOSCHANGING:
+{
+ HWND parent = GetParent(hwnd);
+ if (parent) infoPtr->hwndNotify = parent;
+ return DefWindowProcW(hwnd, uMsg, wParam, lParam);
+}
What if an application subclasses tab and doesn't pass WM_WINDOWPOSCHANGING
to the original proc?
Then it will fail just like it does currently, and will work for the
rest of the applications.
Why not just call GetParent() every time when a
notification is being sent?
Because it won't work if you create a control without WS_CHILD.
Re: [PATCH 4/5] winemac: Implement rudimentary support for system tray icons as Mac status items.
On 3/22/2013 10:08, Ken Thomases wrote: On Mar 21, 2013, at 3:41 PM, C.W. Betts wrote: On Mar 18, 2013, at 3:22 PM, Ken Thomases wrote: On Mar 18, 2013, at 4:04 PM, Charles Davis wrote: At this point, though, I'm wondering if it wouldn't just be easier to have Explorer draw the balloons itself à la Windows XP. Probably. I also considered using an NSPopover anchored to the status item. Again, that's 10.7+ functionality. We could implement it and do compile- and runtime checks to make sure that the functions aren't called in unsupported OS X versions. However, this would probably add fragmentation, and I doubt Alexandre would be happy with this. I think he'd be OK with some variant of this. He might not want two implementations, one using NSPopover on 10.7+ and another for 10.6. I don't know how he'd feel about notification balloons only being implemented on 10.7+. Although I'm not sure how important notification balloons are in the grand scheme of things, anyway. I guess users will let us know if they miss them (unless somebody is inspired to take on that particular corner of unimplemented functionality beforehand). I might be wrong but I think it was some .net installer that wanted user to click on this balloon popup to complete, on the end of installation process it was just waiting for such user input. Cheers, Ken
Re: msxml3: Embed user/password to uri used to create a moniker (try2)
On 3/15/2013 21:43, Nikolay Sivov wrote: try2: fixed builder leak Please ignore this one, it removes too much, I have a better version.
Re: [2/2] wmvcore: Stub implementation of IWMMetadataEditor interface (try2)
On 3/11/2013 16:36, Jeff Latimer wrote:
+if (!ppv) return E_INVALIDARG;
You don't need this most likely.
+IUnknown_AddRef((IUnknown*)*ppv);
A cleaner way is to AddRef original iface, but that's a matter of taste.
+static ULONG WINAPI WMCreateEditor_AddRef(IWMMetadataEditor *iface)
+{
+FIXME("iface=%p\n", iface);
+return 2;
+}
+
+static ULONG WINAPI WMCreateEditor_Release(IWMMetadataEditor *iface)
+{
+FIXME("iface=%p\n", iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return 1;
+}
What's this supposed to mean?
+HRESULT WINAPI WMCreateEditor_flush(IWMMetadataEditor *iface)
+{
+FIXME("iface=%p\n", iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}
+
+HRESULT WINAPI WMCreateEditor_close(IWMMetadataEditor *iface)
+{
+FIXME("iface=%p\n", iface);
+HeapFree(GetProcessHeap(), 0, iface);
+return S_OK;
+}
I don't think so.
HRESULT WINAPI WMCreateEditor(IWMMetadataEditor **editor)
{
+IWMMetadataEditor *This;
FIXME("(%p): stub\n", editor);
-*editor = NULL;
+This = HeapAlloc(GetProcessHeap(), 0, sizeof(IWMMetadataEditor));
+if (!This) return E_OUTOFMEMORY;
-return E_NOTIMPL;
+This->lpVtbl = &Editor_vtable;
+*editor = This;
+
+return S_OK;
}
That's not how it's done, there's plenty of examples already.
-};
+}
That change is ok I think.
Re: [1/2] include: Implement part of nserror.h
On 3/11/2013 13:41, Jeff Latimer wrote: On 11/03/13 17:43, Nikolay Sivov wrote: On 3/11/2013 10:27, Jeff Latimer wrote: --- include/nserror.h | 226 ++ 1 file changed, 226 insertions(+) create mode 100644 include/nserror.h +#ifndef _NSERROR_H +#define _NSERROR_H This doesn't look right. This is the same as the SDK, atleast to my eye. Is there something in particular that appears wrong? See how it's done in existing headers. +#define NS_S_CALLPENDING _HRESULT_TYPEDEF_(0x000DL) This is usually done with a suffix. I have done this as the SDK does it. I thought that was the way we liked it. Sorry, I meant to say 'without a suffix'. See how _HRESULT_TYPEDEF_() is used in winerror.h for example. + +#endif Please add a comment here. Ok.
Re: [1/2] include: Implement part of nserror.h
On 3/11/2013 10:27, Jeff Latimer wrote: --- include/nserror.h | 226 ++ 1 file changed, 226 insertions(+) create mode 100644 include/nserror.h +#ifndef _NSERROR_H +#define _NSERROR_H This doesn't look right. +#define NS_S_CALLPENDING_HRESULT_TYPEDEF_(0x000DL) This is usually done with a suffix. + +#endif Please add a comment here.
Re: comctl32/listview: fix overwriting of item values
On 3/9/2013 10:03, Daniel Jelinski wrote: Makes sense, thanks. I briefly searched for such cases and LISTVIEW_FindItemW() seems to be broken the same way.
Re: msxml3: Implement IXMLParser Get/Set Factory
On 3/6/2013 09:11, Alistair Leslie-Hughes wrote: Hi, Changelog: msxml3: Implement IXMLParser Get/Set Factory You also need to release factory reference on parser release it seems. Best Regards Alistair Leslie-Hughes
Re: gdiplus/tests: Skip the tests if unable to load the TIFF image.
On 3/4/2013 19:30, Dmitry Timoshkov wrote: Francois Gouget wrote: The goal of the Wine tests is to document the Windows behavior that Windows applications expect. Skipping a test because your VM is broken doesn't qualify as a documentation of Windows behaviour. Why is that broken? Tests results could be submitted by anyone running Windows with or without updates. Currently test just returns, with this patch it prints a possible reason. What's a problem? I don't think that you have resources and intention to have Windows VMs with all possible pre/post SP/hotfixes configurations. Having just one regualrly updated one should be more than enough.
Re: comctl32/listview: fix mouse message sequences
On 2/24/2013 17:52, Daniel Jelinski wrote: 2013/2/24 Nikolay Sivov : This doesn't look very clean. I mean invoking *up handler from a *down one. Is this something that could be resolved with message loop like rbutton dragging was fixed? TrackMouse contains the message loop. It returns FALSE when it receives LBUTTONUP (or any other button message), Okay, so why not let corresponding message handler to process it normally? so calling LButtonUp here is almost exactly what happened before. I'm not sure if inlining the call would help here. Regards, Daniel
Re: [1/3] gdiplus: Add some tests for ARGB conversions.
On 2/25/2013 09:29, Dmitry Timoshkov wrote: It seems your system time is an hour ahead. Probably old timezone info?
Re: A new demangler test?
On 2/24/2013 15:13, Eric Pouech wrote: Le 24/02/2013 10:18, Nikolay Sivov a écrit : On 2/24/2013 10:07, Max TenEyck Woodbury wrote: On 02/23/2013 02:54 AM, Eric Pouech wrote: Le 21/02/2013 14:33, Max TenEyck Woodbury a écrit : Would it be appropriate to add a test to the name demangler that: 1) Scans all '.dll' and '.spec' files for mangled names, and 2) Tries to decode those names. 3) Prints the mangled and decoded names and where they occur. Success would be that all names decode without the decoder blowing up or failing. adding tests for demangler is good but you just need to have a set of mangled/unmangled strings for the test as core demangler in msvcrt, test should be added here... (but grabbing the mangled strings shall be left out of the test itself) A+ One of the important aspects of name demangling is that it should work on _all_ the names in the current system. The current test does demangle a list of known strings, but that list was incomplete with respect to all the features used in real names the last time I dug into the details (which was none too recently). Scanning for all the real mangled names not only makes sure that the demangler is working properly, it also assures that all the names used are properly formed and decode to their intended values. I don't see a point to extend test data that much, it's enough to add some most common cases, and add more when something breaks because of it. Otherwise it doesn't deserve that much of attention imho. By the way, do you see something broken or it's just something you want to work on? there are lots of broken stuff (in msvcrt), and demangler in winedump is ante-deluvian (and doesn't share its code with msvcrt) A+ Okay, don't know if winedump part is of any use, I mean it tries to provide a sane C name for decorated names, and usually such names are hand-crafted anyway later. Msvcrt being broken is important of course (at least for dbghelp).
Re: comctl32/listview: fix mouse message sequences
On 2/24/2013 01:33, Daniel Jelinski wrote:
@@ -10178,6 +10139,19 @@ static LRESULT LISTVIEW_LButtonDown(LISTVIEW_INFO
*infoPtr, WORD wKey, INT x, IN
if (infoPtr->dwLvExStyle & LVS_EX_ONECLICKACTIVATE)
if(lvHitTestInfo.iItem != -1)
notify_itemactivate(infoPtr,&lvHitTestInfo);
+if (LISTVIEW_TrackMouse(infoPtr, pt))
+{
+NMLISTVIEW nmlv;
+
+ZeroMemory(&nmlv, sizeof(nmlv));
+nmlv.iItem = lvHitTestInfo.iItem;
+nmlv.ptAction = infoPtr->ptClickPos;
+
+notify_listview(infoPtr, LVN_BEGINDRAG, &nmlv);
+infoPtr->bDragging = TRUE;
+}
+else
+LISTVIEW_LButtonUp(infoPtr,0,x,y);
This doesn't look very clean. I mean invoking *up handler from a *down
one. Is this something that could be resolved with message loop like
rbutton dragging was fixed?
Re: A new demangler test?
On 2/24/2013 10:07, Max TenEyck Woodbury wrote: On 02/23/2013 02:54 AM, Eric Pouech wrote: Le 21/02/2013 14:33, Max TenEyck Woodbury a écrit : Would it be appropriate to add a test to the name demangler that: 1) Scans all '.dll' and '.spec' files for mangled names, and 2) Tries to decode those names. 3) Prints the mangled and decoded names and where they occur. Success would be that all names decode without the decoder blowing up or failing. adding tests for demangler is good but you just need to have a set of mangled/unmangled strings for the test as core demangler in msvcrt, test should be added here... (but grabbing the mangled strings shall be left out of the test itself) A+ One of the important aspects of name demangling is that it should work on _all_ the names in the current system. The current test does demangle a list of known strings, but that list was incomplete with respect to all the features used in real names the last time I dug into the details (which was none too recently). Scanning for all the real mangled names not only makes sure that the demangler is working properly, it also assures that all the names used are properly formed and decode to their intended values. I don't see a point to extend test data that much, it's enough to add some most common cases, and add more when something breaks because of it. Otherwise it doesn't deserve that much of attention imho. By the way, do you see something broken or it's just something you want to work on?
Re: comctl32/treeview: Protect TVM_GETITEM from invalid item pointers
On 2/22/2013 14:09, Alexandre Julliard wrote:
Nikolay Sivov writes:
@@ -2101,9 +2102,17 @@ TREEVIEW_GetItemT(const TREEVIEW_INFO *infoPtr,
LPTVITEMEXW tvItem, BOOL isW)
{
if (!item) return FALSE;
-TRACE("got item from different tree %p, called from %p\n", item->infoPtr, infoPtr);
-infoPtr = item->infoPtr;
-if (!TREEVIEW_ValidItem(infoPtr, item)) return FALSE;
+__TRY
+{
+infoPtr = item->infoPtr;
+TRACE("got item from different tree %p, called from %p\n",
item->infoPtr, infoPtr);
+if (!TREEVIEW_ValidItem(infoPtr, item)) return FALSE;
You can't return from a try block.
Ah, thanks.
Also have you verified that this is not just masking some other Wine
bug?
Honestly, I have not. I only looked at +treeview log attached to bug.
Will try to figure out what's going on exactly.
Re: comctl32/listview: fix selection in ownerdata listview
On 2/21/2013 11:01, Daniel Jelinski wrote: Hello Nikolay, I didn't have time to write and test a proper fix yet, and I'm not sure when I will. I thought this hack may be better than what we have now. Sure, I get it. The problem is that it's not a correct way to fix that. Such hack is ok to attach to a bug report to test with, but it shouldn't be committed. If you want to fix this, please do. If not, I'll get back to it later. I do not plan to look at this right now. Anyway thanks for looking at this code and fixing other things. Regards, Daniel 2013/2/21, Nikolay Sivov : What's a problem in fixing this properly? This change could break something potentially, it was disabled for a purpose of fixing some real application as I remember.
Re: comctl32/listview: fix selection in ownerdata listview
On 2/21/2013 02:19, Daniel Jelinski wrote: /* disable per item notifications on LVS_OWNERDATA style FIXME: single LVN_ODSTATECHANGED should be used */ bOldChange = infoPtr->bDoChangeNotify; -if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->bDoChangeNotify = FALSE; +/*if (infoPtr->dwStyle & LVS_OWNERDATA) infoPtr->bDoChangeNotify = FALSE;*/ What's a problem in fixing this properly? This change could break something potentially, it was disabled for a purpose of fixing some real application as I remember.
Re: fusion: return correct error for null public token
> + ok(hr == FUSION_E_PRIVATE_ASM_DISALLOWED, "Expected S_OK, got %08x\n", hr); You could just strip excepted value message part to avoid typos like that. On Fri, Feb 1, 2013 at 8:10 AM, Alistair Leslie-Hughes < leslie_alist...@hotmail.com> wrote: > Hi, > The other fusion patch is superseded by this one. > > Changelog: > fusion: return correct error for null public token > > > Best Regards >Alistair Leslie-Hughes > > > > > >
Re: fusion: Allow null to be the value of the public key
On 1/31/2013 12:35, Alistair Leslie-Hughes wrote:
Hi,
Changelog:
fusion: Allow null to be the value of the public key
+static const WCHAR nullpublickey[] = {
+
'm','s','c','o','r','l','i','n',',','v','e','r','s','i','o','n','=','0',
+
'p','u','b','l','i','c','K','e','y','T','o','k','e','n','=','n','u','l',
+'c','u','l','t','u','r','e','=','n','e','u','t','r','a','l',0};
You have 'nul' here, not 'null'.
