Re: include[1/2]: add a strcmpW-equivalent function usable in tests.
2010/2/22 Dmitry Timoshkov dmi...@codeweavers.com:
Mikołaj Zalewski miko...@zalewski.pl wrote:
+/* strcmpW is avaiable for tests compiled under Wine, but not in standalone
+ * builds under Windows, so we reimplement it under a different name. */
+static inline int winetest_strcmpW( const WCHAR *str1, const WCHAR *str2 )
+{
+ while (*str1 (*str1 == *str2)) { str1++; str2++; }
+ return *str1 - *str2;
+}
'inline' should not be used in Wine tests, and existing 'inline's should be
removed since that's not a valid modifier for MSVC, and we can't include
config.h in tests.
Inline is already present in include/wine/test.h - in wine_dbgstr_w.
To compile tests under Visual C++, I use #define inline or add this
define to project properties.
Mikołaj
Re: includes: Fix IsEqualPropertyKey definitions
Maarten Lankhorst m.b.lankho...@gmail.com writes: @@ -48,7 +48,11 @@ #endif #ifndef IsEqualPropertyKey +#ifdef __cplusplus #define IsEqualPropertyKey(a,b) (((a).pid == (b).pid) IsEqualIID((a).fmtid,(b).fmtid)) +#else +#define IsEqualPropertyKey(a,b) (((a)-pid == (b)-pid) IsEqualIID((a)-fmtid,(b)-fmtid)) +#endif This doesn't match the PSDK. Where does this come from? -- Alexandre Julliard julli...@winehq.org
Re: [PATCH] shell32: Allow copy operation to overwrite an existing write protected file + tests
Christian Costa titan.co...@wanadoo.fr writes:
@@ -627,6 +627,14 @@ static DWORD SHNotifyCopyFileW(LPCWSTR src, LPCWSTR
dest, BOOL bFailIfExists)
TRACE((%s %s %s)\n, debugstr_w(src), debugstr_w(dest), bFailIfExists
? failIfExists : );
+if (PathFileExistsW(dest))
+{
+ /* Destination file may already exist with read only attribute */
+ DWORD attribs = GetFileAttributesW(dest);
+ if (IsAttrib(attribs, FILE_ATTRIBUTE_READONLY))
+SetFileAttributesW(dest, attribs = ~ FILE_ATTRIBUTE_READONLY);
+}
+
The PathFileExistsW call is redundant.
--
Alexandre Julliard
julli...@winehq.org
Re: [PATCH 3/5] d3dx9_36: Implement stubbed ID3DXConstantTable interface
On 21 February 2010 22:30, Christian Costa titan.co...@wanadoo.fr wrote: +if (IsEqualGUID(riid, IID_IUnknown) || +IsEqualGUID(riid, IID_ID3DXConstantTable)) ID3DXConstantTable extends ID3DXBuffer. +ERR(Interface %s not found\n, debugstr_guid(riid)); Not implementing a random interface an application asks for is hardly an internal error. You could argue for making it a FIXME, but I think WARN is most appropriate.
Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets)
I think tests would be appropriate.
Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets)
Sure. Having tests is better but in this case there is no chance for regression. I usually write tests for my personnal pursose or to compare behaviour against real windows or native dll. I don't think lack of tests in this case should prevent integration. Tests can be added later if needed. That said, if it is really a problem, I can add some basic ones. But I don't think so. Message du 22/02/10 13:06 De : Henri Verbeet A : wine-devel@winehq.org Copie à : Objet : Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets) I think tests would be appropriate.
Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets)
On 22 February 2010 13:43, Christian Costa titan.co...@wanadoo.fr wrote: Sure. Having tests is better but in this case there is no chance for regression. I usually write tests for my personnal pursose or to compare behaviour against real windows or native dll. You write tests to prove your implementation is correct, and to prevent it from breaking in the future. The first thing that looks suspicious is that the documentation claims ppData and pSizeInBytes are allowed to be NULL, while your implementation doesn't allow that. The other thing that looks immediately suspicious is that your return value for pSizeInBytes is inconsistent with the size of the returned data. In terms of style, you calculate the comment size twice, while you could just as easily store it in a variable. Casting to LPCVOID is superfluous. *pSizeInBytes is unsigned, so should use %u in the format string. (I also have some reservations about your naming conventions, but if you can get them past Alexandre they're mostly your own problem.)
Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets)
Wrt the correctness of the behaviour about these parameters, it seems the documentation is not something we should rely on. That's why it's better to write test. So until there is an app which rely on this particular behaviour, this does not worth the trouble. It's not from me. This is something we can hear often on wine devel. Thanks for the review. I will update my patch accordingly. Wrt the naming conventions, Alexandre already complains about msdn style. Unfortunately about the p prefix, it's not clearly stated. More generally I would say that over-reviewing is not a good thing. It does not bring much in term of quality and slow down development. Thrust is the base of a community. Just for the history, I just started doing some stuff on this dll and was surprised that many code have been written which didn't get in. I make a big difference between a self contained dll for which the native one works and another. I make also a big difference between modifying a working code and writing code from scratch. That said. There is not need for a flame war or whatever. Christian Message du 22/02/10 14:20 De : Henri Verbeet A : Christian Costa Copie à : wine-devel@winehq.org Objet : Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets) On 22 February 2010 13:43, Christian Costa wrote: Sure. Having tests is better but in this case there is no chance for regression. I usually write tests for my personnal pursose or to compare behaviour against real windows or native dll. You write tests to prove your implementation is correct, and to prevent it from breaking in the future. The first thing that looks suspicious is that the documentation claims ppData and pSizeInBytes are allowed to be NULL, while your implementation doesn't allow that. The other thing that looks immediately suspicious is that your return value for pSizeInBytes is inconsistent with the size of the returned data. In terms of style, you calculate the comment size twice, while you could just as easily store it in a variable. Casting to LPCVOID is superfluous. *pSizeInBytes is unsigned, so should use %u in the format string. (I also have some reservations about your naming conventions, but if you can get them past Alexandre they're mostly your own problem.)
Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets)
On 22 February 2010 15:15, Christian Costa titan.co...@wanadoo.fr wrote: More generally I would say that over-reviewing is not a good thing. It does not bring much in term of quality and slow down development. Thrust is the base of a community. Sure, but that trust has to be justified. I can certainly stop looking at your patches, I have enough other stuff to do, but I doubt that's going to speed up getting them accepted. It's perhaps also worth noting that I've fixed plenty of bugs that could have easily been avoided if the original author of the code had been more careful, or had bothered to write tests. Seriously, the general principle is that you add a test when you implement some new functionality or fix a bug, unless that's not feasible for some reason. That's certainly not the case here, writing tests for this function is trivial.
Trouble compiling Wine tests with Cygwin
I have a VB6 application that requires oledb32 to convert from DBTYPE_VARIANT to DBTYPE_BYTES, and fails with builtin oledb32 since conversions to DBTYPE_BYTES are unimplemented. I set up a VirtualBox VM with Windows XP and the latest Cygwin in order to add a oledb32 test to show me what should be done for these conversions to DBTYPE_BYTES. However, I have trouble when running make test on dlls/oledb32/tests. I get the following: ../../../tools/winegcc/winegcc -B../../../tools/winebuild --sysroot=../../.. convert.o marshal.o convert_i.o testlist.o -o oledb32_test.exe ../../../libs/port/libwine_port.a -loleaut32 -lole32 -luser32 -lgdi32 -ladvapi32 -lkernel32 oledb32_test.BkIKxE.s: Assembler messages: oledb32_test.BkIKxE.s:5: Error: junk at end of line, first unrecognized character is `' oledb32_test.BkIKxE.s:14: Error: unknown pseudo-op: `.hidden' oledb32_test.BkIKxE.s:69: Error: unknown pseudo-op: `.hidden' oledb32_test.BkIKxE.s:74: Error: junk at end of line, first unrecognized character is `' oledb32_test.BkIKxE.s:76: Error: junk at end of line, first unrecognized character is `-' winebuild: /usr/bin/as.exe failed with status 256 winegcc: ../../../tools/winebuild/winebuild.exe failed make: *** [oledb32_test.exe] Error 2 This is the version of as that comes with the latest cygwin: GNU assembler (GNU Binutils) 2.19.51.20090704 Copyright 2008 Free Software Foundation, Inc. This program is free software; you may redistribute it under the terms of the GNU General Public License version 3 or later. This program has absolutely no warranty. This assembler was configured for a target of `i686-cygwin'. I have searched in www.winehq.org but could not find anything relevant on my problem. I have also tried to save the assembler file by using -Wb,--save-temps but I cannot make it work.
Re: includes: Fix IsEqualPropertyKey definitions
Hi AJ, 2010/2/22 Alexandre Julliard julli...@winehq.org: Maarten Lankhorst m.b.lankho...@gmail.com writes: @@ -48,7 +48,11 @@ #endif #ifndef IsEqualPropertyKey +#ifdef __cplusplus #define IsEqualPropertyKey(a,b) (((a).pid == (b).pid) IsEqualIID((a).fmtid,(b).fmtid)) +#else +#define IsEqualPropertyKey(a,b) (((a)-pid == (b)-pid) IsEqualIID((a)-fmtid,(b)-fmtid)) +#endif This doesn't match the PSDK. Where does this come from? In C, REFPROPERTYKEY becomes PROPERTYKEY*, in C++ it's PROPERTYKEY , so IsEqualPropertyKey will need a different macro. The psdk doesn't define this properly for the C case, but since I need it in C code I need it defined, unless you want me to do IsEqualPropertyKey(*key1, *key2), but even that wouldn't work because of the IsEqualIID.. Cheers, Maarten.
Re: includes: Fix IsEqualPropertyKey definitions
Maarten Lankhorst m.b.lankho...@gmail.com writes: This doesn't match the PSDK. Where does this come from? In C, REFPROPERTYKEY becomes PROPERTYKEY*, in C++ it's PROPERTYKEY , so IsEqualPropertyKey will need a different macro. The psdk doesn't define this properly for the C case, but since I need it in C code I need it defined, unless you want me to do IsEqualPropertyKey(*key1, *key2), but even that wouldn't work because of the IsEqualIID.. That doesn't match the PSDK I have, and yes it has a C definition that is different from the C++ one, but is also different from yours. -- Alexandre Julliard julli...@winehq.org
Re: includes: Fix IsEqualPropertyKey definitions
Hi AJ, 2010/2/22 Alexandre Julliard julli...@winehq.org: Maarten Lankhorst m.b.lankho...@gmail.com writes: This doesn't match the PSDK. Where does this come from? In C, REFPROPERTYKEY becomes PROPERTYKEY*, in C++ it's PROPERTYKEY , so IsEqualPropertyKey will need a different macro. The psdk doesn't define this properly for the C case, but since I need it in C code I need it defined, unless you want me to do IsEqualPropertyKey(*key1, *key2), but even that wouldn't work because of the IsEqualIID.. That doesn't match the PSDK I have, and yes it has a C definition that is different from the C++ one, but is also different from yours. Mine doesn't seem to have a C definition at all, but from the devpropdef one I assume the C version takes a PROPERTYKEY as parameter and not a PROPERTYKEY*? Cheers, Maarten.
Re: includes: Fix IsEqualPropertyKey definitions
Maarten Lankhorst m.b.lankho...@gmail.com writes: That doesn't match the PSDK I have, and yes it has a C definition that is different from the C++ one, but is also different from yours. Mine doesn't seem to have a C definition at all, but from the devpropdef one I assume the C version takes a PROPERTYKEY as parameter and not a PROPERTYKEY*? Yes. -- Alexandre Julliard julli...@winehq.org
Re: Trouble compiling Wine tests with Cygwin
On Mon, Feb 22, 2010 at 11:03 AM, Alex Villacís Lasso a_villa...@palosanto.com wrote: I have a VB6 application that requires oledb32 to convert from DBTYPE_VARIANT to DBTYPE_BYTES, and fails with builtin oledb32 since conversions to DBTYPE_BYTES are unimplemented. I set up a VirtualBox VM with Windows XP and the latest Cygwin in order to add a oledb32 test to show me what should be done for these conversions to DBTYPE_BYTES. However, I have trouble when running make test on dlls/oledb32/tests. I get the following: It's not quite the answer to your question, but why not use mingw on your native OS and run the mingw created exe on windows? -- -Austin
Re: Trouble compiling Wine tests with Cygwin
El 22/02/10 12:41, Austin English escribió: On Mon, Feb 22, 2010 at 11:03 AM, Alex Villacís Lasso a_villa...@palosanto.com wrote: I have a VB6 application that requires oledb32 to convert from DBTYPE_VARIANT to DBTYPE_BYTES, and fails with builtin oledb32 since conversions to DBTYPE_BYTES are unimplemented. I set up a VirtualBox VM with Windows XP and the latest Cygwin in order to add a oledb32 test to show me what should be done for these conversions to DBTYPE_BYTES. However, I have trouble when running make test on dlls/oledb32/tests. I get the following: It's not quite the answer to your question, but why not use mingw on your native OS and run the mingw created exe on windows? Thanks. I will try that. However, the Cygwin-on-Windows method used to work, and I have tried some test patches with it, so it is surprising for me that it does not work anymore.
[PATCH] server: Fix console create function
Check create_event return value.
---
server/console.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/server/console.c b/server/console.c
index a79dc67..3758a71 100644
--- a/server/console.c
+++ b/server/console.c
@@ -294,7 +294,7 @@ static struct object *create_console_input( struct thread*
renderer )
console_input-win = 0;
console_input-event = create_event( NULL, NULL, 0, 1, 0, NULL );
-if (!console_input-history || !console_input-evt)
+if (!console_input-history || !console_input-evt ||
!console_input-event)
{
release_object( console_input );
return NULL;
--
1.6.0.4
Re: [PATCH] shlwapi/tests: skip SHCreateStreamOnFileA/W configurations not supported on Win98 SE.
On 02/23/2010 01:28 AM, Reece Dunn wrote:
+if (ret == E_INVALIDARG) /* Win98 SE */ {
+skip(Not supported\n);
+return;
+}
Hi Reece,
Could you turn that (and the other one) into a win_skip() ?
--
Cheers,
Paul.
Re: dns-sd dll implementation -- Needs work
C.W. Betts computer...@hotmail.com wrote: What do you guys think? it's a wrapper for multicast DNS, aka Bonjour®. Looks like this is not a part of Windows or PSDK, so Wine shouldn't include it either. -- Dmitry.
Re: [PATCH] shlwapi/tests: skip SHCreateStreamOnFileA/W configurations not supported on Win98 SE.
On 23 February 2010 07:33, Paul Vriens paul.vriens.w...@gmail.com wrote:
On 02/23/2010 01:28 AM, Reece Dunn wrote:
+ if (ret == E_INVALIDARG) /* Win98 SE */ {
+ skip(Not supported\n);
+ return;
+ }
Hi Reece,
Could you turn that (and the other one) into a win_skip() ?
Done, thanks.
- Reece
Re: [PATCH 4/5] d3dx9_36: Implement D3DXFindShaderComment (based on code from Luis Busquets)
Henri Verbeet a écrit : On 22 February 2010 15:15, Christian Costa titan.co...@wanadoo.fr wrote: More generally I would say that over-reviewing is not a good thing. It does not bring much in term of quality and slow down development. Thrust is the base of a community. Sure, but that trust has to be justified. I can certainly stop looking at your patches, I have enough other stuff to do, but I doubt that's going to speed up getting them accepted. It's perhaps also worth noting that I've fixed plenty of bugs that could have easily been avoided if the original author of the code had been more careful, or had bothered to write tests. Seriously, the general principle is that you add a test when you implement some new functionality or fix a bug, unless that's not feasible for some reason. That's certainly not the case here, writing tests for this function is trivial. The goal was to implement the IConstantTable interface. I intended to write tests at this level along with new patch series for this interface. D3DXFindShaderComment was not needed directly by a particular app but just to be reused by D3DXGetShaderConstantTableEx as the next patch shows. But since it's better to do it in this patch, I will add some tests to it then.
