Re: [PATCH] dinput: Mark internal symbols as hidden visibility
On Mon, May 16, 2011 at 08:26:08PM -0600, Vitaliy Margolen wrote: > On 05/13/2011 08:51 AM, Marcus Meissner wrote: > >-const DIDATAFORMAT c_dfDIJoystick = { > >+DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIJoystick = { > > >-const DIDATAFORMAT c_dfDIJoystick2 = { > >+DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIJoystick2 = { > > >-const DIDATAFORMAT c_dfDIMouse = { > >+DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIMouse = { > > >-const DIDATAFORMAT c_dfDIMouse2 = { > >+DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIMouse2 = { > > >-const DIDATAFORMAT c_dfDIKeyboard = { > >+DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIKeyboard = { > > These are public exports see include/dinput.h:1988. However, they are not exported via the .spec file, so they will not get visible... How should they be exported? Is there a static library in Windows? Ciao, Marcus
Re: Trailing white space and other pedantry.
On 05/16/2011 09:39 PM, Ken Thomases wrote: On May 16, 2011, at 7:42 PM, max wrote: On 05/16/2011 01:00 PM, André Hentschel wrote: I understand that white space only patches will not be applied. Will they be applied if they accompany other corrections? If one would otherwise have modified a line in the course of doing development, then one can and should fix an issue like trailing whitespace on that line. If the line would not have been changed except for the whitespace issue, then it shouldn't be changed. Also, please address the other questions. Well, he might not have known the answer to or had an opinion about the other questions. Should I submit patches against simple errors in macro definition formatting? There are some places where my program catches mismatches that SOME other compilers might ignore. There are also some places in the wine headers where macros are redefined differently from the headers provided by the system or compilers I have installed. There are also cases where the macros are defined differently by different wine headers. Because these problems may depend on my system configuration, I believe it might not be possible to simply patch these definitions match the configuration I have. The problems CAN be fixed simply by adding #undef before each redefinition, but I think a review of these changes might be in order. They seem to occur in batches, with none in most headers and multiple messes in others. Am I understanding correctly that these are whitespace difference again? Maybe you could provide an example of what you mean. To my knowledge the mismatches you have identified have never actually caused problems. Until they do, I expect that fixes for them won't be accepted. Also, when they do, that will probably help guide which of your proposed solutions is preferred. FYI: Scanning (C) source file 'dlls/atl/atl_ax.c'. --Warning: Trailing white space at dlls/atl/atl_ax.c:991. --Warning: Trailing white space at dlls/atl/atl_ax.c:1005. --Warning: Trailing white space at dlls/atl/atl_ax.c:1027. --Warning: Trailing white space at dlls/atl/atl_ax.c:1042. --Warning: Trailing white space at dlls/atl/atl_ax.c:1131. --Warning: Trailing white space at dlls/atl/atl_ax.c:1141. --Warning: Trailing white space at dlls/atl/atl_ax.c:1245. Scanning (C) source file 'dlls/atl/registrar.c'. --Warning: Trailing white space at dlls/atl/registrar.c:206. --Warning: Trailing white space at dlls/atl/registrar.c:213. --Warning: Trailing white space at dlls/atl/registrar.c:243. --Warning: Trailing white space at dlls/atl/registrar.c:380. --Warning: Trailing white space at dlls/atl/registrar.c:525. --Warning: Trailing white space at dlls/atl/registrar.c:597. Should I submit each correction as a separate patch, separate patches accompanying separate bug reports, combined patches for a given header or combined patch and bug report? Hard to say without an illustration of what you're talking about. In the simple cases, they would produce compiler warnings, not functional issues. In other cases, there are differences that I expect do not show up because the wine project does not use the macro, but could cause application programmers grief. For example: Scanning (C) source file 'dlls/advapi32/service.c'. ==Error: Attempt to change the definition of macro '__attribute__' at line 88 of include/wine/exception.h ignored. Originally defined at /usr/include/sys/cdefs.h:203 Old:' ( xyz ) ' New:' ( x ) ' Scanning (C) source file 'dlls/avicap32/avicap32_main.c'. ==Error: Attempt to change the definition of macro 'IOC_OUT' at line 697 of include/winsock.h ignored. Originally defined at /usr/include/asm-generic/ioctl.h:91 Old:' ( _IOC_READ << _IOC_DIRSHIFT ) ' New:' 1073741824 ' ==Error: Attempt to change the definition of macro 'IOC_IN' at line 698 of include/winsock.h ignored. Originally defined at /usr/include/asm-generic/ioctl.h:90 Old:' ( _IOC_WRITE << _IOC_DIRSHIFT ) ' New:' 2147483648 ' ==Error: Attempt to change the definition of macro 'IOC_INOUT' at line 699 of include/winsock.h ignored. Originally defined at /usr/include/asm-generic/ioctl.h:92 Old:' ( ( _IOC_WRITE | _IOC_READ ) << _IOC_DIRSHIFT ) ' New:' ( IOC_IN | IOC_OUT ) ' ==Error: Attempt to change the definition of macro '_IO' at line 701 of include/winsock.h ignored. Originally defined at /usr/include/asm-generic/ioctl.h:74 Old:' ( type , nr ) _IOC ( _IOC_NONE , ( type ) , ( nr ) , 0 ) ' New:' ( x , y ) ( IOC_VOID | ( ( x ) << 8 ) | ( y ) ) ' ==Error: Attempt to change the definition of macro '_IOR' at line 702 of include/winsock.h ignored. Originally defined at /usr/include/asm-generic/ioctl.h:75 Old:' ( type , nr , size ) _IOC ( _IOC_READ , ( type ) , ( nr ) , ( _IOC_TYPECHECK ( size ) ) ) ' New:' ( x , y , t ) ( IOC_OUT | ( ( ( UINT ) sizeof ( t ) & IOCPARM_M
Re: [PATCH] dinput: Mark internal symbols as hidden visibility
On 05/13/2011 08:51 AM, Marcus Meissner wrote: -const DIDATAFORMAT c_dfDIJoystick = { +DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIJoystick = { -const DIDATAFORMAT c_dfDIJoystick2 = { +DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIJoystick2 = { -const DIDATAFORMAT c_dfDIMouse = { +DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIMouse = { -const DIDATAFORMAT c_dfDIMouse2 = { +DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIMouse2 = { -const DIDATAFORMAT c_dfDIKeyboard = { +DECLSPEC_HIDDEN const DIDATAFORMAT c_dfDIKeyboard = { These are public exports see include/dinput.h:1988. Vitaliy.
Re: Trailing white space and other pedantry.
On May 16, 2011, at 7:42 PM, max wrote: > On 05/16/2011 01:00 PM, André Hentschel wrote: >> Am 16.05.2011 18:34, schrieb m...@mtew.isa-geek.net: >>> From: Max TenEyck Woodbury >>> >>> I have been working on the documentation extraction problem and code >>> analysis problem. >>> >>> I have made some progress with the program I am writing to do this. It >>> reads the same files as c2man.pl does, but does a more detailed parse >>> of the code. In fact, it even reads and processes all the include >>> files. It is far from complete at this time, but it does produce >>> interesting warnings. >>> >>> One of the warnings reports lines with trailing while space. It has >>> turned up quite a few places where this occurs. If I understand the >>> preferred style, there should not be trailing white space. The >>> question I have is what should I do with them. I can either fix the >>> files or turn that particular warning off. So far, I have been fixing >>> the files on a local copy of the repository. >>> >>> Should I turn the fixes into patches and submit them, or just keep them >>> to myself? >>> >> No, white space only changes will not be accepted. >> Your are right about the preferred style, but most likely the regarding code >> is a bit old. >> new code should never have trailing white space. >> The best reason is "git blame" to see who wrote the code, i think that makes >> it clear. > > I understand that white space only patches will not be applied. Will > they be applied if they accompany other corrections? If one would otherwise have modified a line in the course of doing development, then one can and should fix an issue like trailing whitespace on that line. If the line would not have been changed except for the whitespace issue, then it shouldn't be changed. > Also, please address the other questions. Well, he might not have known the answer to or had an opinion about the other questions. > Should I submit patches against simple errors in macro definition > formatting? There are some places where my program catches mismatches > that SOME other compilers might ignore. > > There are also some places in the wine headers where macros are > redefined differently from the headers provided by the system or > compilers I have installed. There are also cases where the macros are > defined differently by different wine headers. Because these problems > may depend on my system configuration, I believe it might not be > possible to simply patch these definitions match the configuration I > have. The problems CAN be fixed simply by adding #undef before each > redefinition, but I think a review of these changes might be in order. > They seem to occur in batches, with none in most headers and multiple > messes in others. Am I understanding correctly that these are whitespace difference again? Maybe you could provide an example of what you mean. To my knowledge the mismatches you have identified have never actually caused problems. Until they do, I expect that fixes for them won't be accepted. Also, when they do, that will probably help guide which of your proposed solutions is preferred. > Should I submit each correction as a separate patch, separate patches > accompanying separate bug reports, combined patches for a given header > or combined patch and bug report? Hard to say without an illustration of what you're talking about. Regards, Ken
Re: Trailing white space and other pedantry.
On 05/16/2011 01:00 PM, André Hentschel wrote: Am 16.05.2011 18:34, schrieb m...@mtew.isa-geek.net: From: Max TenEyck Woodbury I have been working on the documentation extraction problem and code analysis problem. I have made some progress with the program I am writing to do this. It reads the same files as c2man.pl does, but does a more detailed parse of the code. In fact, it even reads and processes all the include files. It is far from complete at this time, but it does produce interesting warnings. One of the warnings reports lines with trailing while space. It has turned up quite a few places where this occurs. If I understand the preferred style, there should not be trailing white space. The question I have is what should I do with them. I can either fix the files or turn that particular warning off. So far, I have been fixing the files on a local copy of the repository. Should I turn the fixes into patches and submit them, or just keep them to myself? No, white space only changes will not be accepted. Your are right about the preferred style, but most likely the regarding code is a bit old. new code should never have trailing white space. The best reason is "git blame" to see who wrote the code, i think that makes it clear. I understand that white space only patches will not be applied. Will they be applied if they accompany other corrections? Also, please address the other questions. Should I submit patches against simple errors in macro definition formatting? There are some places where my program catches mismatches that SOME other compilers might ignore. There are also some places in the wine headers where macros are redefined differently from the headers provided by the system or compilers I have installed. There are also cases where the macros are defined differently by different wine headers. Because these problems may depend on my system configuration, I believe it might not be possible to simply patch these definitions match the configuration I have. The problems CAN be fixed simply by adding #undef before each redefinition, but I think a review of these changes might be in order. They seem to occur in batches, with none in most headers and multiple messes in others. Should I submit each correction as a separate patch, separate patches accompanying separate bug reports, combined patches for a given header or combined patch and bug report? Max
Re: Regression in ioctl
Am 16.05.2011 22:49, schrieb Susan Cragin: > This popped up today, and prevented me from opening my program. > > susan@ubuntu:~/.wine/drive_c/Program > Files/Nuance/NaturallySpeaking10/Program$ wine natspeak > fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for > IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS > fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 > fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for > IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS > fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 > fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for > IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS > fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 > fixme:reg:RegSetKeySecurity :(0x54,4,0xb05a50): stub > fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for > IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS > fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 > susan@ubuntu:~/.wine/drive_c/Program > Files/Nuance/NaturallySpeaking10/Program$ > > I had just compiled today's git. > wine-1.3.20-44-gddad22d > > Is there a regression test in my future? > Hi, When did you compiled the last time? yesterday or some month ago? -- Best Regards, André Hentschel
Regression in ioctl
This popped up today, and prevented me from opening my program. susan@ubuntu:~/.wine/drive_c/Program Files/Nuance/NaturallySpeaking10/Program$ wine natspeak fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 fixme:reg:RegSetKeySecurity :(0x54,4,0xb05a50): stub fixme:mountmgr:harddisk_ioctl returning zero-filled buffer for IOCTL_VOLUME_GET_VOLUME_DISK_EXTENTS fixme:mountmgr:harddisk_ioctl unsupported ioctl 74080 susan@ubuntu:~/.wine/drive_c/Program Files/Nuance/NaturallySpeaking10/Program$ I had just compiled today's git. wine-1.3.20-44-gddad22d Is there a regression test in my future?
Re: Trailing white space and other pedantry.
Am 16.05.2011 18:34, schrieb m...@mtew.isa-geek.net: > From: Max TenEyck Woodbury > > I have been working on the documentation extraction problem and code > analysis problem. > > I have made some progress with the program I am writing to do this. It > reads the same files as c2man.pl does, but does a more detailed parse > of the code. In fact, it even reads and processes all the include > files. It is far from complete at this time, but it does produce > interesting warnings. > > One of the warnings reports lines with trailing while space. It has > turned up quite a few places where this occurs. If I understand the > preferred style, there should not be trailing white space. The > question I have is what should I do with them. I can either fix the > files or turn that particular warning off. So far, I have been fixing > the files on a local copy of the repository. > > Should I turn the fixes into patches and submit them, or just keep them > to myself? No, whitespace only changes will not be accepted. Your are right about the preferred style, but most likely the regarding code is a bit old. new code should never have trailing whitespace. The best reason is "git blame" to see who wrote the code, i think that makes it clear. -- Best Regards, André Hentschel
Trailing white space and other pedantry.
From: Max TenEyck Woodbury I have been working on the documentation extraction problem and code analysis problem. I have made some progress with the program I am writing to do this. It reads the same files as c2man.pl does, but does a more detailed parse of the code. In fact, it even reads and processes all the include files. It is far from complete at this time, but it does produce interesting warnings. One of the warnings reports lines with trailing while space. It has turned up quite a few places where this occurs. If I understand the preferred style, there should not be trailing white space. The question I have is what should I do with them. I can either fix the files or turn that particular warning off. So far, I have been fixing the files on a local copy of the repository. Should I turn the fixes into patches and submit them, or just keep them to myself? Another problem the program warns about is incompatible macro redefinitions. In a few places the differences are just white space, and I have made local fixes. In other places, the differences are more substantial and there are two basic options available. The more difficult solution is to change the definitions in wine to make them compatible with the rest of the system. This could cause problems for people with different system configurations. The other solution is to simply #undef the macros before redefining them. Should I turn the simple fixes into patches and submit them? Should I submit the other problems as bug reports? Max
Re: [5/5] comctl32/treeview: Don't send TVN_ITEMEXPANDING when item has no children
On Mon, May 16, 2011 at 2:11 PM, Alexandre Julliard wrote: > Nikolay Sivov writes: > >> Fix for http://bugs.winehq.org/show_bug.cgi?id=23591 >> >> From 3bb38e13558c6e06efbb33b7e24cc36c4ae5973f Mon Sep 17 00:00:00 2001 >> From: Nikolay Sivov >> Date: Sat, 14 May 2011 20:03:40 +0400 >> Subject: [PATCH 5/5] Don't send TVN_ITEMEXPANDING when item has no children > > It doesn't work here: Mmm, ok. I'll test this thing with native comctl32 too. > > ../../../tools/runtest -q -P wine -M explorerframe.dll -T ../../.. -p > explorerframe_test.exe.so nstc.c && touch nstc.ok > nstc.c:1668: Test failed: Got 0x80070057 > nstc.c:1933: Test failed: Got event 6, count 0 > nstc.c:1934: Test failed: Got event 7, count 0 > nstc.c:1935: Test failed: No event. > nstc.c:1949: Test failed: Got event 4, count 0 > nstc.c:1999: Test failed: Got event 6, count 0 > nstc.c:2000: Test failed: No event. > nstc.c:2001: Test failed: Got event 7, count 0 > nstc.c:2010: Test failed: Item not expanded. > nstc.c:2030: Test failed: itemstate is 0x > nstc.c:2080: Test succeeded inside todo block: Got event 6, count 0 > nstc.c:2081: Test succeeded inside todo block: Got event 7, count 0 > nstc.c:2082: Test succeeded inside todo block: Got event 12, count 0 > nstc.c:2090: Test succeeded inside todo block: itemstate is 0x Bright side of it. > nstc.c:2163: Test failed: itemstate is 0x > nstc.c:2216: Test failed: itemstate is 0x0004 > nstc.c:2228: Test failed: itemstate is 0x0004 > nstc.c:2283: Test failed: Got event 6, count 0 > nstc.c:2284: Test failed: No event. > nstc.c:2285: Test failed: Got event 7, count 0 > nstc.c:2290: Test failed: Got (0x80004005) > nstc.c:2291: Test failed: psi is (nil) > make: *** [nstc.ok] Error 22 > > -- > Alexandre Julliard > julli...@winehq.org > > >
Re: d3d: avoid implicit conversion warnings from identical enumeration types D3DPOOL and WINED3DPOOL (Clang)
2011/5/16 Frédéric Delanoy : > Is there in that case no point in getting rid of those warnings by > adding conversion functions between those related-but-not-identical > enums? > Well, in case of something like IDirect3DCubeTexture9Impl_GetType() there's no point in asking wined3d at all, the implementation should just be "return D3DRTYPE_CUBETEXTURE;". In case of something like IDirect3D9Impl_CheckDeviceFormat() we should have a conversion function.
Re: d3d: avoid implicit conversion warnings from identical enumeration types D3DPOOL and WINED3DPOOL (Clang)
2011/5/16 Henri Verbeet : > 2011/5/16 Frédéric Delanoy : >> Indeed. I'm checking a couple of other automatic non-checked enum >> conversions, and those can be problematic; e.g. D3DRESOURCETYPE and >> WINED3DRESOURCETYPE are not identical but are autoconverted without >> any kind of check. >> > The resource type handling in e.g. d3d9 is really ugly, but should be > safe in practice. IDirect3DCubeTexture9Impl_GetType() should only ever > return D3DRTYPE_CUBETEXTURE for example. > Is there in that case no point in getting rid of those warnings by adding conversion functions between those related-but-not-identical enums? Couldn't this help detect invalid enum values quicker instead of passing them around unchecked? Frédéric Delanoy
Re: d3d: avoid implicit conversion warnings from identical enumeration types D3DPOOL and WINED3DPOOL (Clang)
2011/5/16 Frédéric Delanoy : > Indeed. I'm checking a couple of other automatic non-checked enum > conversions, and those can be problematic; e.g. D3DRESOURCETYPE and > WINED3DRESOURCETYPE are not identical but are autoconverted without > any kind of check. > The resource type handling in e.g. d3d9 is really ugly, but should be safe in practice. IDirect3DCubeTexture9Impl_GetType() should only ever return D3DRTYPE_CUBETEXTURE for example.
Re: [5/5] comctl32/treeview: Don't send TVN_ITEMEXPANDING when item has no children
Nikolay Sivov writes: > Fix for http://bugs.winehq.org/show_bug.cgi?id=23591 > > From 3bb38e13558c6e06efbb33b7e24cc36c4ae5973f Mon Sep 17 00:00:00 2001 > From: Nikolay Sivov > Date: Sat, 14 May 2011 20:03:40 +0400 > Subject: [PATCH 5/5] Don't send TVN_ITEMEXPANDING when item has no children It doesn't work here: ../../../tools/runtest -q -P wine -M explorerframe.dll -T ../../.. -p explorerframe_test.exe.so nstc.c && touch nstc.ok nstc.c:1668: Test failed: Got 0x80070057 nstc.c:1933: Test failed: Got event 6, count 0 nstc.c:1934: Test failed: Got event 7, count 0 nstc.c:1935: Test failed: No event. nstc.c:1949: Test failed: Got event 4, count 0 nstc.c:1999: Test failed: Got event 6, count 0 nstc.c:2000: Test failed: No event. nstc.c:2001: Test failed: Got event 7, count 0 nstc.c:2010: Test failed: Item not expanded. nstc.c:2030: Test failed: itemstate is 0x nstc.c:2080: Test succeeded inside todo block: Got event 6, count 0 nstc.c:2081: Test succeeded inside todo block: Got event 7, count 0 nstc.c:2082: Test succeeded inside todo block: Got event 12, count 0 nstc.c:2090: Test succeeded inside todo block: itemstate is 0x nstc.c:2163: Test failed: itemstate is 0x nstc.c:2216: Test failed: itemstate is 0x0004 nstc.c:2228: Test failed: itemstate is 0x0004 nstc.c:2283: Test failed: Got event 6, count 0 nstc.c:2284: Test failed: No event. nstc.c:2285: Test failed: Got event 7, count 0 nstc.c:2290: Test failed: Got (0x80004005) nstc.c:2291: Test failed: psi is (nil) make: *** [nstc.ok] Error 22 -- Alexandre Julliard julli...@winehq.org
Re: d3d: avoid implicit conversion warnings from identical enumeration types D3DPOOL and WINED3DPOOL (Clang)
2011/5/16 Stefan Dösinger : > I'm not quite sure about that patch. I sent a bunch of patches to add float<- >>integer casts myself to silence some msvc warnings. WINED3DPOOL in the way it > currently is will go away sooner or later when we transition to a d3d10-style > resource model. Well there's no loss of precision/type change in this patch. Those are still plain (integer) enums. However, I can understand casting may be risky if one of the enum structures changes, but again it's not like there was some kind of "value validity checking" earlier. > Still the problem will remain with other structures and types. Indeed. I'm checking a couple of other automatic non-checked enum conversions, and those can be problematic; e.g. D3DRESOURCETYPE and WINED3DRESOURCETYPE are not identical but are autoconverted without any kind of check. Say you have enum A { A_FOO, A_BAR, A_BAZ } and enum B { B_FOO, B_BAR } How are you supposed to convert 'enum A' BAZ to 'enum B'? Probably issueing a fixme in the default clause, but then what would be a default return value? enum B B_from_A(elem) { switch( case A_FOO: return B_FOO; case A_BAR: return B_BAR; default: FIXME("Unhandled elem type %#x.\n", elem); return ??? } } Some enums, like D3DFORMAT specify a D3DFMT_UNKNOWN as first (0 valued) first enum value, which is returned in the "default" cause as well. Is this the way it should be done? a standard in wine coding? i.e. should previous example be changed to: enum A { A_UNKNOWN, A_FOO, A_BAR, A_BAZ } enum B { B_UNKNOWN, B_FOO, B_BAR } enum B B_from_A(elem) { switch( case A_FOO: return B_FOO; case A_BAR: return B_BAR; default: FIXME("Unhandled elem type %#x.\n", elem); return B_UNKNOWN; } } If it isn't a standard in wine, would that be a viable solution? Frédéric