June 2017 - Wine-Devel - list.winehq.org

Re: [PATCH] shell32: Implement FCIDM_SHVIEW_CREATELINK

by Aric Stewart

On 6/2/17 10:49 AM, Nikolay Sivov wrote: > On 02.06.2017 17:24, Aric Stewart wrote: >> >> Presently the wine file explorer has a create shortcut entry that >> does nothing. This implements the FCIDM_SHVIEW_CREATELINK command. >> >> Signed-off-by: Aric Stewart <aric(a)codeweavers.com> >> --- >> dlls/shell32/shlview_cmenu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 50 insertions(+) >> > > >> + if (!SUCCEEDED(CoCreateInstance( &CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, >> + &IID_IShellLinkW, (LPVOID*)&shelllink))) >> + { >> + ERR("couldn't create ShellLink object\n"); >> + return; >> + } > > This should be FAILED(). You can use IShellLink_Constructor() instead of > going through generic object creation. > > > + SHGetSpecialFolderPathW(NULL, root, CSIDL_STARTMENU, TRUE); >> + lnkfile=HeapAlloc(GetProcessHeap(), 0, (lstrlenW(root)+1+lstrlenW(wszFilename)+5) * sizeof(*lnkfile)); >> + lstrcpyW(lnkfile, root); >> + lstrcatW(lnkfile, backslashW); >> + lstrcatW(lnkfile, wszFilename); >> + lstrcatW(lnkfile, lnkW); > > Why is it created in Start Menu dir? I think usual behavior is to create > it in target directory. Create the link in the same directory as the original program exists? That seems counter intuitive to me. I could seen an argument for the desktop instead of the start menu. But again I have not used windows myself in years. -aric

June 5, 2017

Re: [PATCH 2/4] ntdll: Provide more accurate virtual memory counters.

by Alex Henrie

Out of curiosity, is there a particular application that depends on this behavior? It could be argued that reporting the value from the native operating system is better because it tells what's really happening in the process. -Alex

June 5, 2017

Re: [PATCH] msxml3: Remove CRs in domtext_put_data and add them in domtext_get_xml.

by Nikolay Sivov

This needs more tests, for different msxml object versions, and for different input document. It's possible that get_xml() returns original document, so test loading non trivial document is necessary. I remember looking at this some time ago, not for this specific report but in general, and my conclusion was that the only way to generate matching document is to duplicate node dumping in our code.

June 5, 2017

Re: [PATCH v7 12/26] x86/insn-eval: Add utility functions to get segment descriptor base address and limit

by Ricardo Neri

On Wed, 2017-05-31 at 18:58 +0200, Borislav Petkov wrote: > On Fri, May 05, 2017 at 11:17:10AM -0700, Ricardo Neri wrote: > > With segmentation, the base address of the segment descriptor is needed > > to compute a linear address. The segment descriptor used in the address > > computation depends on either any segment override prefixes in the > > instruction or the default segment determined by the registers involved > > in the address computation. Thus, both the instruction as well as the > > register (specified as the offset from the base of pt_regs) are given as > > inputs, along with a boolean variable to select between override and > > default. > > ... > > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > > index f46cb31..c77ed80 100644 > > --- a/arch/x86/lib/insn-eval.c > > +++ b/arch/x86/lib/insn-eval.c > > @@ -476,6 +476,133 @@ static struct desc_struct *get_desc(unsigned short sel) > > } > > > > /** > > + * insn_get_seg_base() - Obtain base address of segment descriptor. > > + * @regs: Structure with register values as seen when entering kernel mode > > + * @insn: Instruction structure with selector override prefixes > > + * @regoff: Operand offset, in pt_regs, of which the selector is needed > > + * > > + * Obtain the base address of the segment descriptor as indicated by either > > + * any segment override prefixes contained in insn or the default segment > > + * applicable to the register indicated by regoff. regoff is specified as the > > + * offset in bytes from the base of pt_regs. > > + * > > + * Return: In protected mode, base address of the segment. Zero in for long > > + * mode, except when FS or GS are used. In virtual-8086 mode, the segment > > + * selector shifted 4 positions to the right. -1L in case of > > + * error. > > + */ > > +unsigned long insn_get_seg_base(struct pt_regs *regs, struct insn *insn, > > + int regoff) > > +{ > > + struct desc_struct *desc; > > + unsigned short sel; > > + enum segment_register seg_reg; > > + > > + seg_reg = resolve_seg_register(insn, regs, regoff); > > + if (seg_reg == SEG_REG_INVAL) > > + return -1L; > > + > > + sel = get_segment_selector(regs, seg_reg); > > + if ((short)sel < 0) > > I guess it would be better if that function returned a signed short so > you don't have to cast it here. (You're casting it to an unsigned long > below anyway.) Yes, this make sense. I will make this change. > > > + return -1L; > > + > > + if (v8086_mode(regs)) > > + /* > > + * Base is simply the segment selector shifted 4 > > + * positions to the right. > > + */ > > + return (unsigned long)(sel << 4); > > + > > ... > > > +static unsigned long get_seg_limit(struct pt_regs *regs, struct insn *insn, > > + int regoff) > > +{ > > + struct desc_struct *desc; > > + unsigned short sel; > > + unsigned long limit; > > + enum segment_register seg_reg; > > + > > + seg_reg = resolve_seg_register(insn, regs, regoff); > > + if (seg_reg == SEG_REG_INVAL) > > + return 0; > > + > > + sel = get_segment_selector(regs, seg_reg); > > + if ((short)sel < 0) > > Ditto. Here as well. > > > + return 0; > > + > > + if (user_64bit_mode(regs) || v8086_mode(regs)) > > + return -1L; > > + > > + if (!sel) > > + return 0; > > + > > + desc = get_desc(sel); > > + if (!desc) > > + return 0; > > + > > + /* > > + * If the granularity bit is set, the limit is given in multiples > > + * of 4096. When the granularity bit is set, the least 12 significant > > the 12 least significant bits > > > + * bits are not tested when checking the segment limits. In practice, > > + * this means that the segment ends in (limit << 12) + 0xfff. > > + */ > > + limit = get_desc_limit(desc); > > + if (desc->g) > > + limit <<= 12 | 0x7; > > That 0x7 doesn't look like 0xfff - it shifts limit by 15 instead. You > can simply write it like you mean it: > > limit = (limit << 12) + 0xfff; You are right, this wrong. I will implement as you mention. Thanks and BR, Ricardo

June 3, 2017

Re: [v2 PATCH 2/3] mfplat: Implement MFCreateMediaType

by Alistair Leslie-Hughes

Hi Henri, > >If everyone is going to comment, I might as well join in. >What is the purpose of the typedef? The purpose is to make a type. For example without it every time we needed to use it we would have to use "struct _mfmediatype name", but with the typedef we can shorthand it to "mfmediatype name". Best Regards Alistair.

June 3, 2017

Re: [PATCH] shell32: Implement FCIDM_SHVIEW_CREATELINK

by Nikolay Sivov

On 02.06.2017 17:24, Aric Stewart wrote: > > Presently the wine file explorer has a create shortcut entry that > does nothing. This implements the FCIDM_SHVIEW_CREATELINK command. > > Signed-off-by: Aric Stewart <aric(a)codeweavers.com> > --- > dlls/shell32/shlview_cmenu.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 50 insertions(+) > > + if (!SUCCEEDED(CoCreateInstance( &CLSID_ShellLink, NULL, CLSCTX_INPROC_SERVER, > + &IID_IShellLinkW, (LPVOID*)&shelllink))) > + { > + ERR("couldn't create ShellLink object\n"); > + return; > + } This should be FAILED(). You can use IShellLink_Constructor() instead of going through generic object creation. > + SHGetSpecialFolderPathW(NULL, root, CSIDL_STARTMENU, TRUE); > + lnkfile=HeapAlloc(GetProcessHeap(), 0, (lstrlenW(root)+1+lstrlenW(wszFilename)+5) * sizeof(*lnkfile)); > + lstrcpyW(lnkfile, root); > + lstrcatW(lnkfile, backslashW); > + lstrcatW(lnkfile, wszFilename); > + lstrcatW(lnkfile, lnkW); Why is it created in Start Menu dir? I think usual behavior is to create it in target directory. > + if (!SUCCEEDED(IShellLinkW_QueryInterface(shelllink, &IID_IPersistFile, (LPVOID*)&persistfile))) > + { > + ERR("couldn't get IPersistFile interface\n"); > + IShellLinkW_Release(shelllink); > + HeapFree(GetProcessHeap(), 0, lnkfile); > + return; > + } FAILED() again, but really it can't fail. Also please LPVOID -> void*.

June 2, 2017

Re: kernel32: Fix WriteConsoleOutputCharacter[AW] spec file entries

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > @@ -1612,8 +1612,8 @@ > @ stdcall WriteConsoleInputW(long ptr long ptr) > @ stdcall WriteConsoleOutputA(long ptr long long ptr) > @ stdcall WriteConsoleOutputAttribute(long ptr long long ptr) > -@ stdcall WriteConsoleOutputCharacterA(long ptr long long ptr) > -@ stdcall WriteConsoleOutputCharacterW(long ptr long long ptr) > +@ stdcall WriteConsoleOutputCharacterA(long str long long ptr) > +@ stdcall WriteConsoleOutputCharacterW(long wstr long long ptr) It's questionable to use string types on potentially non null-terminated strings. -- Alexandre Julliard julliard(a)winehq.org

June 2, 2017

Re: [PATCH 2/8] comcat: label ordinals

by Alexandre Julliard

Austin English <austinenglish(a)gmail.com> writes: > Signed-off-by: Austin English <austinenglish(a)gmail.com> > --- > dlls/comcat/comcat.spec | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/dlls/comcat/comcat.spec b/dlls/comcat/comcat.spec > index 5b5f7b06cb..781b07437c 100644 > --- a/dlls/comcat/comcat.spec > +++ b/dlls/comcat/comcat.spec > @@ -1,4 +1,4 @@ > -@ stdcall -private DllCanUnloadNow() > -@ stdcall -private DllGetClassObject(ptr ptr ptr) ole32.DllGetClassObject > -@ stdcall -private DllRegisterServer() > -@ stdcall -private DllUnregisterServer() > +1 stdcall -private DllGetClassObject(ptr ptr ptr) ole32.DllGetClassObject > +2 stdcall -private DllCanUnloadNow() > +3 stdcall -private DllRegisterServer() > +4 stdcall -private DllUnregisterServer() It doesn't make much sense to define ordinals for such functions. -- Alexandre Julliard julliard(a)winehq.org

June 2, 2017

Re: [PATCH] user32: Keep tracking if we just created the menu.

by Huw Davies

On Thu, Jun 01, 2017 at 08:27:22AM -0500, Zebediah Figura wrote: > The problem is that the menu which appears on clicking the title bar, and > contains elements such as minimize/restore/move/etc., is destroyed as soon > as it is created. To replicate this, start notepad (or anything else) in a > virtual window, then try to click the window icon in the upper left hand > corner. Ok, I see what you mean. Fixing this is turning out to be somewhat non-trivial, so leave it with me and I'll hopefully have something next week. Thanks, Huw.

June 2, 2017

Re: [PATCH] fusion: use correct size for ZeroMemory (GCC7)

by Marcus Meissner

On Fri, Jun 02, 2017 at 04:58:50PM +0800, Dmitry Timoshkov wrote: > Marcus Meissner <meissner(a)suse.de> wrote: > > > diff --git a/dlls/fusion/tests/asmname.c b/dlls/fusion/tests/asmname.c > > index 5fb14a4829..21cd4874ed 100644 > > --- a/dlls/fusion/tests/asmname.c > > +++ b/dlls/fusion/tests/asmname.c > > @@ -358,7 +358,7 @@ static void test_assembly_name_props_line(IAssemblyName *name, > > to_widechar(expect, vals[i].val); > > > > size = MAX_PATH; > > - ZeroMemory(str, MAX_PATH); > > + ZeroMemory(str, sizeof(str)); > > hr = IAssemblyName_GetProperty(name, i, str, &size); > > to_multibyte(val, str); > > Same problem with IAssemblyName_GetProperty() and 'size' argumenet as > for the previous patch. I am not sure documentation and reality agree on this. If I pass in size in bytes and then later on change it back to character count the testsuite starts to fail with 2* factors. Our implementation and upstream(?) seems to return character count on some of those properties. Ciao, Marcus

June 2, 2017