Re: [PATCH 2/2] comctl32: Send CBEM_DELETEITEM when CB_RESETCONTENT is received
On 7/22/2011 08:26, Jay Yang wrote:
On 07/22/2011 12:19 AM, Nikolay Sivov wrote:
On 7/22/2011 06:59, Jay Yang wrote:
diff --git a/dlls/comctl32/comboex.c b/dlls/comctl32/comboex.c
index 3402a71..4888519 100644
--- a/dlls/comctl32/comboex.c
+++ b/dlls/comctl32/comboex.c
@@ -1557,6 +1557,11 @@ static void COMBOEX_ResetContent (COMBOEX_INFO *infoPtr)
item = infoPtr->items;
while (item) {
+NMCOMBOBOXEXW nmcit;
+memset (&nmcit.ceItem, 0, sizeof(nmcit.ceItem));
+nmcit.ceItem.mask=~0;
+COMBOEX_CopyItem (item,&nmcit.ceItem);
+COMBOEX_NotifyItem (infoPtr, CBEN_DELETEITEM,&nmcit);
next = item->next;
COMBOEX_FreeText (item);
Free (item);
First of all no need for memset probably, and it's common rule I think (well
for me at least) to
avoid unneeded initialization.
At the very least I need to set the pszText field to null because
COMBOEX_CopyItem looks at that to
determine whether to copy the array. Mostly I was just following the other
examples in the file.
I see, let it be that way then.
As we have message to delete a single item, it's probably time to think about
factoring out item
deletion helper (that sends notification too).
Actually it seems strange for me that this control uses list to store item
data, cause messages
are clearly index based. So first thing I'd like to see with that is to change
item storage to use
DPA array as others controls do. But if you don't have time for that now, I
could try to do it
myself.
Yeah, I'm not exactly focused on controls right now, I'm just trying to fix a
memory leak that I
found in explorer because it doesn't get the CBEM_DELETEITEM notification, so
if you know a more
correct way to do this, by all means do it. On the otherhand, I do sort of want
to get some kind of
fix in sometime soon for this because it causes a memory leak in explorer.
Okay, so I'll take a look at item storage for it in a couple of days.
--
Jay Yang
Re: [PATCH 2/2] comctl32: Send CBEM_DELETEITEM when CB_RESETCONTENT is received
On 07/22/2011 12:19 AM, Nikolay Sivov wrote:
> On 7/22/2011 06:59, Jay Yang wrote:
>> diff --git a/dlls/comctl32/comboex.c b/dlls/comctl32/comboex.c
>> index 3402a71..4888519 100644
>> --- a/dlls/comctl32/comboex.c
>> +++ b/dlls/comctl32/comboex.c
>> @@ -1557,6 +1557,11 @@ static void COMBOEX_ResetContent (COMBOEX_INFO
>> *infoPtr)
>>
>> item = infoPtr->items;
>> while (item) {
>> +NMCOMBOBOXEXW nmcit;
>> +memset (&nmcit.ceItem, 0, sizeof(nmcit.ceItem));
>> +nmcit.ceItem.mask=~0;
>> +COMBOEX_CopyItem (item,&nmcit.ceItem);
>> +COMBOEX_NotifyItem (infoPtr, CBEN_DELETEITEM,&nmcit);
>> next = item->next;
>> COMBOEX_FreeText (item);
>> Free (item);
> First of all no need for memset probably, and it's common rule I think (well
> for me at least) to
> avoid unneeded initialization.
At the very least I need to set the pszText field to null because
COMBOEX_CopyItem looks at that to
determine whether to copy the array. Mostly I was just following the other
examples in the file.
> As we have message to delete a single item, it's probably time to think about
> factoring out item
> deletion helper (that sends notification too).
>
> Actually it seems strange for me that this control uses list to store item
> data, cause messages
> are clearly index based. So first thing I'd like to see with that is to
> change item storage to use
> DPA array as others controls do. But if you don't have time for that now, I
> could try to do it
> myself.
Yeah, I'm not exactly focused on controls right now, I'm just trying to fix a
memory leak that I
found in explorer because it doesn't get the CBEM_DELETEITEM notification, so
if you know a more
correct way to do this, by all means do it. On the otherhand, I do sort of want
to get some kind of
fix in sometime soon for this because it causes a memory leak in explorer.
--
Jay Yang
Re: [PATCH 2/2] comctl32: Send CBEM_DELETEITEM when CB_RESETCONTENT is received
On 7/22/2011 06:59, Jay Yang wrote:
diff --git a/dlls/comctl32/comboex.c b/dlls/comctl32/comboex.c
index 3402a71..4888519 100644
--- a/dlls/comctl32/comboex.c
+++ b/dlls/comctl32/comboex.c
@@ -1557,6 +1557,11 @@ static void COMBOEX_ResetContent (COMBOEX_INFO *infoPtr)
item = infoPtr->items;
while (item) {
+NMCOMBOBOXEXW nmcit;
+memset (&nmcit.ceItem, 0, sizeof(nmcit.ceItem));
+nmcit.ceItem.mask=~0;
+COMBOEX_CopyItem (item,&nmcit.ceItem);
+COMBOEX_NotifyItem (infoPtr, CBEN_DELETEITEM,&nmcit);
next = item->next;
COMBOEX_FreeText (item);
Free (item);
First of all no need for memset probably, and it's common rule I think
(well for me at least) to avoid unneeded initialization.
As we have message to delete a single item, it's probably time to think
about factoring out item deletion helper (that sends notification too).
Actually it seems strange for me that this control uses list to store
item data, cause messages are clearly index based. So first thing I'd
like to see with that is to change item storage to use DPA array as
others controls do. But if you don't have time for that now, I could try
to do it myself.
Re: [PATCH 1/2] comctl32: Add tests for CB_RESETCONTENT
On 7/22/2011 06:59, Jay Yang wrote: Tests that CBEM_DELETEITEM notifications are received when CB_RESETCONTENT is sent to a comboex control. --- dlls/comctl32/tests/comboex.c | 28 1 files changed, 28 insertions(+), 0 deletions(-) Better way to test that is to use message sequence test. Look at listview test to see how WM_NOTIFY/id pairs are logged.
Wininet set_cookie must handle not only = but also =
Hello, while running uTorrent 3 I see the following fixme several times:
fixme:wininet:set_cookie Unknown additional option L"expires =
Sat,23-Jul-2011 02:20:13 GMT"
After searching the source code I realized the expires parameter is
already implemented but wine finds the entry names only if there's no
space between the parameter name and the equal sign, here is a small
piece of wininet.c around line 440 and 470.
...
static const WCHAR szExpires[] = {'e','x','p','i','r','e','s','=',0}
...
else if (strncmpiW(ptr, szExpires, 8) == 0)
...
After searching msdn I found this article with an example that uses spaces:
http://msdn.microsoft.com/en-us/library/aa385326%28v=vs.85%29.aspx
bReturn = InternetSetCookie(TEXT("http://www.adventure_works.com";), NULL,
TEXT("TestData = Test; expires = Sat,01-Jan-2000 00:00:00 GMT"));
Looks like the cookie above is valid. I also found this other article
telling about the data format:
http://msdn.microsoft.com/en-us/library/aa384321%28v=vs.85%29.aspx
After reading parts of the cookies RFC I found this note:
http://www.ietf.org/rfc/rfc2109.txt
...
NOTE: The syntax above allows whitespace between the attribute and
the = sign.
...
The newer version of the cookie rfc has the very same note:
http://www.ietf.org/rfc/rfc2965.txt
Please, don't bother to answer if this was already noted and is to be
fixed. Also sorry for not pointing a solution. I'll study more on this
later but I can say in advance that windows xp sets the cookie
correctly when there is the space. I guess I'll start by adding tests
for this on the file tests/internet.c
Best wishes,
Bruno
--
universe* god::bigbang (void); //and then it all began...
Winhelp crash in assertion(0) if .hlp file is not found
Hello, I've been in the list for a long time as a reader and after
years I finally managed to get a development computer and restarted
playing with current wine git.
There are about 15 bugs on bugzilla but none of them are related to
this. I'm trying the list first before opening a bug. It's quite
simple to reproduce, although I'm not sure if I can attach the hlp
file here since it's from windows. The crash happens because the help
file needs another help file. There is a button on the screen called
Glossary that needs a file called glossary.hlp. If this file does not
exist wine asks me to point it, if I answer No or yes and then cancel
file selection wine crashes on an assert.
The following lines are displayed on the console:
fixme:winhelp:WINHELP_GetWindowInfo Couldn't find window info for glossary
winhelp.c:270: WINHELP_GetWindowInfo: Assertion `0' failed.
wine: Assertion failed at address 0xb78f0424 (thread 003b), starting debugger...
The following lines are from file winhelp.c, around line 270.
...
if (strcmp(name, "main") != 0)
{
WINE_FIXME("Couldn't find window info for %s\n", name);
assert(0);
return NULL;
}
...
The function WINHELP_OpenHelpWindow does not resist
WINHELP_GetWindowInfo returning NULL. That was why the assert(0) was
added inside it to abort the program execution (as far as I see).
There are 3 calls like this (one on winhelp.c and the others on
macro.c):
...
WINHELP_OpenHelpWindow(HLPFILE_PageByHash, hlpfile, lHash,
WINHELP_GetWindowInfo(hlpfile, wndname), show);
...
My approach to fix this is quite simple: instead of splitting the
above lines on different places and add a check for each of them the
easier way is to simply make OpenHelpWindow resist null pointers and
do not open anything because the user was already warned and asked for
the nonexistent file. The proposed solution (if I may call adding one
"if" a solution) is attached. If anyone has the problematic hlp file
that was the cause to add the assert(0) please send me. Looks like the
assert is there since wine 1.0 according to
http://source.winehq.org/source/programs/winhlp32/winhelp.c?v=wine-1.0
Best wishes,
Bruno
--
universe* god::bigbang (void); //and then it all began...
From 824d59111b704e3db4cb7ff89bc6bf9781ea53be Mon Sep 17 00:00:00 2001
From: Bruno Jesus <00cp...@gmail.com>
Date: Sat, 23 Jul 2011 00:17:47 -0300
Subject: winhelp.c: Resist nonexistent files instead of crashing with assert(0);
---
programs/winhlp32/winhelp.c | 11 +++
1 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/programs/winhlp32/winhelp.c b/programs/winhlp32/winhelp.c
index a43a784..1247f13 100644
--- a/programs/winhlp32/winhelp.c
+++ b/programs/winhlp32/winhelp.c
@@ -22,7 +22,6 @@
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
*/
-#include
#include
#include
#include
@@ -264,12 +263,8 @@ HLPFILE_WINDOWINFO* WINHELP_GetWindowInfo(HLPFILE* hlpfile, LPCSTR name)
if (!lstrcmpi(hlpfile->windows[i].name, name))
return &hlpfile->windows[i];
-if (strcmp(name, "main") != 0)
-{
-WINE_FIXME("Couldn't find window info for %s\n", name);
-assert(0);
-return NULL;
-}
+if (strcmp(name, "main") != 0) return NULL;
+
if (!mwi.name[0])
{
strcpy(mwi.type, "primary");
@@ -882,7 +877,7 @@ BOOL WINHELP_OpenHelpWindow(HLPFILE_PAGE* (*lookup)(HLPFILE*, LONG, ULONG*),
int nCmdShow)
{
WINHELP_WNDPAGE wpage;
-
+if(!wi) return FALSE;
wpage.page = lookup(hlpfile, val, &wpage.relative);
if (wpage.page) wpage.page->file->wRefCount++;
wpage.wininfo = wi;
--
1.7.2.5
Re:
Sorry, this is a duplicate for the [3/4] patch, you can disregard it.
2011/7/21 Lucas Fialho Zawacki :
> ---
> dlls/dinput/joystick.c | 60
> +++-
> 1 files changed, 59 insertions(+), 1 deletions(-)
>
> diff --git a/dlls/dinput/joystick.c b/dlls/dinput/joystick.c
> index b5c028c..2d8a818 100644
> --- a/dlls/dinput/joystick.c
> +++ b/dlls/dinput/joystick.c
> @@ -500,9 +500,67 @@ HRESULT WINAPI
> JoystickWGenericImpl_SetActionMap(LPDIRECTINPUTDEVICE8W iface,
> LPCWSTR lpszUserName,
> DWORD dwFlags)
> {
> + JoystickGenericImpl *This = impl_from_IDirectInputDevice8W(iface);
> + DIDATAFORMAT data_format;
> + DIOBJECTDATAFORMAT *obj_df = NULL;
> + int i, action = 0, num_actions = 0;
> + unsigned int offset = 0;
> +
> FIXME("(%p)->(%p,%s,%08x): semi-stub !\n", iface, lpdiaf,
> debugstr_w(lpszUserName), dwFlags);
>
> - return DI_NOEFFECT;
> + if (This->base.acquired) return DIERR_ACQUIRED;
> +
> + data_format.dwSize = sizeof(data_format);
> + data_format.dwObjSize = sizeof(DIOBJECTDATAFORMAT);
> + data_format.dwFlags = DIDF_RELAXIS;
> + data_format.dwDataSize = lpdiaf->dwDataSize;
> +
> + /* count the actions */
> + for (i=0; i < lpdiaf->dwNumActions; i++)
> + if (IsEqualGUID(&This->base.guid,
> &lpdiaf->rgoAction[i].guidInstance))
> + num_actions++;
> +
> + if (num_actions == 0) return DI_NOEFFECT;
> +
> + This->base.num_actions = num_actions;
> +
> + /* Construct the dataformat and actionmap */
> + obj_df = HeapAlloc(GetProcessHeap(), 0,
> sizeof(DIOBJECTDATAFORMAT)*num_actions);
> + data_format.rgodf = (LPDIOBJECTDATAFORMAT)obj_df;
> + data_format.dwNumObjs = num_actions;
> +
> + This->base.action_map = HeapAlloc(GetProcessHeap(), 0,
> sizeof(ActionMap)*num_actions);
> +
> + for (i = 0; i < lpdiaf->dwNumActions; i++)
> + {
> + if (IsEqualGUID(&This->base.guid,
> &lpdiaf->rgoAction[i].guidInstance))
> + {
> + LPDIDATAFORMAT df = This->base.data_format.wine_df;
> + DWORD inst = DIDFT_GETINSTANCE(lpdiaf->rgoAction[i].dwObjID);
> + DWORD type = DIDFT_GETTYPE(lpdiaf->rgoAction[i].dwObjID);
> + LPDIOBJECTDATAFORMAT obj;
> +
> + if (type == DIDFT_PSHBUTTON) type = DIDFT_BUTTON;
> + if (type == DIDFT_RELAXIS) type = DIDFT_AXIS;
> +
> + obj = dataformat_to_odf_by_type(df, inst, type);
> +
> + memcpy(&obj_df[action], obj, df->dwObjSize);
> +
> + This->base.action_map[action].uAppData =
> lpdiaf->rgoAction[i].uAppData;
> + This->base.action_map[action].offset = offset;
> + obj_df[action].dwOfs = offset;
> + offset += (type & DIDFT_BUTTON) ? 1 : 4;
> +
> + action++;
> + }
> + }
> +
> + IDirectInputDevice8_SetDataFormat(iface, &data_format);
> +
> + HeapFree(GetProcessHeap(), 0, obj_df);
> +
> + return IDirectInputDevice8WImpl_SetActionMap(iface, lpdiaf,
> lpszUserName, dwFlags);
> }
>
> HRESULT WINAPI JoystickAGenericImpl_SetActionMap(LPDIRECTINPUTDEVICE8A iface,
> --
> 1.7.0.4
>
Re: netapi32: add msvcrt to importlibs
Louis Lenders writes: > hi, this fixes bug http://bugs.winehq.org/show_bug.cgi?id=27827 > > Creo Elements expects msvcrt to be loaded, and fails to start. > > Dependancywalker on windows shows netapi32 loads the msvcrt. I guess > when more netapi32 functions will be implemented in future this will > become a "real" necessary dependency It won't help if it's not used, and you'd need to use msvcrt headers too. I don't think it's the right place to fix this. -- Alexandre Julliard julli...@winehq.org
Re: Experimental website for tracking translation issues
On Thu, 21 Jul 2011, Alexandre Julliard wrote: > Francois Gouget writes: > > > I think we can ignore thin non-breaking spaces like everyone else > > (Gnomefr for instance). At least for now. However, based on your > > previous comment and the fines.pdf report, it seems like putting > > non-breaking spaces everywhere in Wine should be safe enough. > > Does anyone object? > > That's only necessary for messages that get wrapped automatically, > i.e. in message boxes. In general you don't want it for things that are > shown on the command line. It sounds like following these rules will be complex which means they won't really be followed. Also for places where it makes no difference I'd prefer to use the typographically correct string. So my preference would be that we use non-breaking spaces everywhere except where they break things, and I'm hoping the exception is either 'never' or 'except in command line applications' which would be simple enough. Do we actually have evidence that regular (non-thin) non-breaking spaces break things on the command line? According to the fines.pdf report they seem to be working everywhere (except in mrxvt which I'll just ignore). I suspect the author did not test any non-UTF8 locale however... http://malaria.perso.sfr.fr/fines/fines.pdf -- Francois Gouget http://fgouget.free.fr/ Advice is what we ask for when we already know the answer but wish we didn't -- Eric Jong
Re: GSoC: dinput8 Action Mapping
On Tue, Jul 19, 2011 at 09:40:37PM -0300, Lucas Zawacki wrote:
> Hey, I've got a couple patches implementing build and setactionmap for
> joysticks and I'd like you guys to give them a look.
The patches look good to my eyes.
This part:
> +if ((lpdiaf->rgoAction[i].dwSemantic & genre) == genre)
still looks strange (as the first genre should be a some kind
of bitmask instead of genre?), but will likely work as-is for now.
> These implementations are very generic and I think I could use them
> for the keyboard and the mouse too. If everything's alright with these
> patches that'll be my next step.
>
> The mapping can be tested this small app I built:
> https://github.com/downloads/lfzawacki/dinput-samples/dolphin.exe
> provided that you have one joystick attached (it will chose the first
> one if there are more).
Works with my Joystick here at least.
Not sure what it should output, A and X button works, the
ranges go to 10/-10 expect for "Down" just going to -5 (might be a issue with
the joystick).
Ciao, Marcus
> 2011/7/5 Lucas Zawacki :
> > Hi guys.
> >
> > Now that all the initial patches are in I have several smaller tasks
> > to work on as listed here
> > http://lfzawacki.heroku.com/wine/published/Road+Map and hopefully
> > they're more straightforward. I've already started working on getting
> > EnumDevicesBySemantics correct with joysticks and the passed flags
> > and, after that, BuildActionMap for the joysticks will follow. Maybe
> > now it's time to discuss how to implement EnumDevicesBySemantics as a
> > crosscall, but I really don't know how to do it (or if it's worth it)
> > and everywhere I look in dinput I see similar cases of this
> > "duplication pattern".
> >
> > Another thing that I've been thinking is that I might as well end up
> > rolling a version of ConfigureDevices
> > (http://msdn.microsoft.com/en-us/library/microsoft.directx_sdk.idirectinput8.idirectinput8.configuredevices%28v=VS.85%29.aspx)
> > because so far I've seen two of the games affected by bug 8754 use it
> > to configure controls. I've not had time to find and test all of them,
> > but if someone on the list knows about other games that use it I'd
> > like to be informed.
> >
> > Last but not least, thanks for Wylda for testing NFSU and keeping bug
> > 8754 informed. I actually can't run the game properly, most likely
> > because of the crappy intel graphics card in my laptop.
> >
> > Anyway, feedback on the tasks is welcome.
> >
> > ---
> > lfz
> >
> From b492c54d5ca7179e0905f63cec183d0920555e1d Mon Sep 17 00:00:00 2001
> From: Lucas Fialho Zawacki
> Date: Tue, 19 Jul 2011 17:52:12 -0300
> Subject: [PATCH 1/4] dinput: EnumDevicesBySemantics enumerating joysticks
> with priority flags
>
> Added an utility function that checks the priority of a device for a given
> mapping. This can be modified later to return priority 2 mappings, if
> necessary.
> ---
> dlls/dinput/dinput_main.c | 50
> 1 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/dlls/dinput/dinput_main.c b/dlls/dinput/dinput_main.c
> index b653307..1d59cdc 100644
> --- a/dlls/dinput/dinput_main.c
> +++ b/dlls/dinput/dinput_main.c
> @@ -317,6 +317,38 @@ void _copy_diactionformatWtoA(LPDIACTIONFORMATA to,
> LPDIACTIONFORMATW from)
> }
> }
>
> +/* _diactionformat_priorityA
> + *
> + * Given a DIACTIONFORMAT structure and a DI genre, returns the enumeration
> + * priority. Joysticks should pass the game genre, and mouse or keyboard
> their
> + * respective DI*_MASK
> + */
> +static DWORD _diactionformat_priorityA(LPDIACTIONFORMATA lpdiaf, DWORD genre)
> +{
> +int i;
> +DWORD priorityFlags = 0;
> +
> +/* If there's at least one action for the device it's priority 1 */
> +for(i=0; i < lpdiaf->dwActionSize; i++)
> +if ((lpdiaf->rgoAction[i].dwSemantic & genre) == genre)
> +priorityFlags |= DIEDBS_MAPPEDPRI1;
> +
> +return priorityFlags;
> +}
> +
> +static DWORD _diactionformat_priorityW(LPDIACTIONFORMATW lpdiaf, DWORD genre)
> +{
> +int i;
> +DWORD priorityFlags = 0;
> +
> +/* If there's at least one action for the device it's priority 1 */
> +for(i=0; i < lpdiaf->dwActionSize; i++)
> +if ((lpdiaf->rgoAction[i].dwSemantic & genre) == genre)
> +priorityFlags |= DIEDBS_MAPPEDPRI1;
> +
> +return priorityFlags;
> +}
> +
>
> /**
> * IDirectInputA_EnumDevices
> */
> @@ -877,7 +909,7 @@ static HRESULT WINAPI
> IDirectInput8AImpl_EnumDevicesBySemantics(
> {
> TRACE(" - checking device %u ('%s')\n", i,
> dinput_devices[i]->name);
>
> -callbackFlags = 0;
> +callbackFlags = _diactionformat_priorityA(lpdiActionFormat,
> lpdiActionFormat->dwGenre);
> /* Default behavior is to enumerate attached game controllers */
> enumSuccess =
> dinput_devic
