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

Re: Review | endproc.c: Implement ProcessPage_OnEndProcessTree

by Akarsha Sehwag

> Although unlikely, HeapReAlloc() can fail and return NULL. It would be > better to handle that possibility. Since increase_list_size() and > process_list_append() are both small functions, I think it's worth > considering merging increase_list_size() into process_list_append(), > since that would probably simplify handling HeapReAlloc() failures. > The comment about HEAP_ZERO_MEMORY in init_process_list() applies here > as well. > I didn't understand the function of the flags in HeapAlloc() and HeapReAlloc(). I read the function RtlAllocateHeap() but I am unable to figure out which flag to use out of HEAP_GENERATE_EXCEPTIONS, HEAP_NO_SERIALIZE and HEAP_ZERO_MEMORY. All other changes done.* Diff file attached. *

April 27, 2017

Re: [4/4] ole32: Fix IDataObject_GetDataHere may not implement

by Huw Davies

On Wed, Apr 26, 2017 at 03:45:50PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From 2c5a9b4df394c6d50bcc24bcc7b489a864224155 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:24:00 +0800 > Subject: [PATCH 4/4] ole32: Fix IDataObject_GetDataHere may not implement > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/ole2impl.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/dlls/ole32/ole2impl.c b/dlls/ole32/ole2impl.c > index a0c4445..5ee031e 100644 > --- a/dlls/ole32/ole2impl.c > +++ b/dlls/ole32/ole2impl.c > @@ -115,6 +115,10 @@ static HRESULT get_storage(IDataObject *data, IStorage *stg, UINT *src_cf, BOOL > med.u.pstg = stg; > med.pUnkForRelease = NULL; > hr = IDataObject_GetDataHere(data, &fmt, &med); > + if (FAILED(hr)) > + { > + hr = IDataObject_GetData(data, &fmt, &med); > + } This can't work. The STGMEDIUM is an [out]-only parameter of _GetData(), meaning that the callee will return a new stg in the STGMEDIUM. If this is indeed called (we need tests to check), then you'd have to copy the returned stg into the original one. Huw.

April 27, 2017

Re: [3/4] ole32: Add init cache code after OleLoad in OleCreateFromDataEx

by Huw Davies

On Wed, Apr 26, 2017 at 03:46:23PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From a96e73f270745f0694ea340eb8119566d75f1117 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:14:36 +0800 > Subject: [PATCH 3/4] ole32: Add init cache code after OleLoad in > OleCreateFromDataEx > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/ole2impl.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/dlls/ole32/ole2impl.c b/dlls/ole32/ole2impl.c > index ccf31fa..a0c4445 100644 > --- a/dlls/ole32/ole2impl.c > +++ b/dlls/ole32/ole2impl.c > @@ -181,6 +181,7 @@ HRESULT WINAPI OleCreateFromDataEx(IDataObject *data, REFIID iid, DWORD flags, > { > HRESULT hr; > UINT src_cf; > + IOleCache* ole_cache; > > FIXME("(%p, %s, %08x, %08x, %d, %p, %p, %p, %p, %p, %p, %p): stub\n", > data, debugstr_guid(iid), flags, renderopt, num_cache_fmts, adv_flags, cache_fmts, > @@ -193,6 +194,11 @@ HRESULT WINAPI OleCreateFromDataEx(IDataObject *data, REFIID iid, DWORD flags, > if(FAILED(hr)) return hr; > > /* FIXME: Init cache */ > + hr = IUnknown_QueryInterface((IUnknown*)*obj, &IID_IOleCache, (void**)&ole_cache); > + if (SUCCEEDED(hr)) > + { > + hr = IOleCache_InitCache(ole_cache, data); > + } You'll need to release the ole_cache. Again we'll need some tests. Huw.

April 27, 2017

Re: [2/4] ole32: Support HBITMAP format in DataCache_GetExtent and DataCache_Draw

by Huw Davies

On Wed, Apr 26, 2017 at 03:44:01PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control > From 91b46bc9da357b76b8028d058a191743e9a31a32 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Wed, 26 Apr 2017 15:05:59 +0800 > Subject: [PATCH 2/4] ole32: Support HBITMAP format in DataCache_GetExtent and > DataCache_Draw > > Fix cann't insert any HBITMAP in RichEdit control > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ole32/datacache.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 52 insertions(+) > > @@ -1908,6 +1926,40 @@ static HRESULT WINAPI DataCache_GetExtent( > > return S_OK; > } > + case CF_BITMAP: > + { > + HDC hdc; > + char dst_bmibuf[FIELD_OFFSET( BITMAPINFO, bmiColors[256] )]; > + BITMAPINFO *dst_info = (BITMAPINFO *)dst_bmibuf; > + BITMAPINFOHEADER *info; > + LONG x_pels_m, y_pels_m; > + > + if (cache_entry->stgmedium.tymed != TYMED_GDI) > + continue; > + > + hdc = GetDC(NULL); > + memset(dst_info, 0, sizeof(char)*FIELD_OFFSET( BITMAPINFO, bmiColors[256] )); > + dst_info->bmiHeader.biSize = sizeof(BITMAPINFOHEADER); > + > + GetDIBits(hdc, cache_entry->stgmedium.u.hBitmap, 0, 1, NULL, dst_info, DIB_RGB_COLORS); > + > + info = &dst_info->bmiHeader; I'm not convinced adding the info variable helps here. Just use dst_info->bmiHeader throughout. Better still drop the 'dst_' and use BITMAPINFO *info = (BITMAPINFO *)bmibuf; > + x_pels_m = info->biXPelsPerMeter; > + y_pels_m = info->biYPelsPerMeter; > + > + /* Size in units of 0.01mm (ie. MM_HIMETRIC) */ > + if (x_pels_m != 0 && y_pels_m != 0) > + { > + lpsizel->cx = info->biWidth * 100000 / x_pels_m; > + lpsizel->cy = info->biHeight * 100000 / y_pels_m; > + } > + else > + { > + lpsizel->cx = info->biWidth * 2540 / GetDeviceCaps( hdc, LOGPIXELSX ); > + lpsizel->cy = info->biHeight * 2540 / GetDeviceCaps( hdc, LOGPIXELSY ); > + } You need a ReleaseDC(0, hdc); > + return S_OK; > + } > } > } > > -- > 2.10.2 > >

April 27, 2017

Re: [1/4] ole32: Implement DataCache_InitCache

by Huw Davies

On Wed, Apr 26, 2017 at 03:42:59PM +0800, Wei Xie wrote: > Fix cann't insert any HBITMAP in RichEdit control Hi, Thanks for working on this. In general this series in going to need some unit tests to be submitted along with it. There are already some for the data-cache in dlls/ole32/tests/ole2.c. The documentation for IOleCache_InitCache() implies that it supposed to be used after first calling IOleCache_Cache() to specify which formats are to be cached, rather than the way you have implemented it. I'd imagine that OleCache_InitCache() would call IDataObject_GetData() on each of the wanted formats and pass that off to IOleCache_SetData(). Anyway, some tests will confirm this. I'll add a couple of comments to the other patches, but the tests need writing first. [Note, it's "can't" or "cannot", not "cann't"]. Huw.

April 27, 2017

Re: [PATCH 5/5] user32/tests: Try to read the dpi from the user key first

by Huw Davies

On Thu, Apr 27, 2017 at 05:17:52AM -0500, 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 > https://testbot.winehq.org/JobDetails.pl?Key=30508 > > Your paranoid android. > > > === w8 (32 bit sysparams) === > sysparams.c:2517: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out > sysparams.c:2528: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 > > === w1064 (32 bit sysparams) === > sysparams.c:204: Test failed: Unexpected WM_DISPLAYCHANGE message > These are failures not related to this patch. Huw.

April 27, 2017

Re: [PATCH 5/5] user32/tests: Try to read the dpi from the user key first

by Marvin

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 https://testbot.winehq.org/JobDetails.pl?Key=30508 Your paranoid android. === w8 (32 bit sysparams) === sysparams.c:2517: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2528: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w1064 (32 bit sysparams) === sysparams.c:204: Test failed: Unexpected WM_DISPLAYCHANGE message

April 27, 2017

Re: [PATCH v2 08/11] d3d11/tests: Add test for state refcounting.

by Marvin

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 https://testbot.winehq.org/JobDetails.pl?Key=30506 Your paranoid android. === build (build) === Patch failed to apply

April 27, 2017

Re: [PATCH 1/5] d3dx9/tests: Get rid of compare_rotation.

by Henri Verbeet

On 26 April 2017 at 23:27, Marvin <testbot(a)winehq.org> wrote: > === build (build) === > Patch failed to apply > This seems to be mostly because the testbot tried to run these before the "Update Wine to latest git" job.

April 27, 2017

Re: [v6 PATCH 12/21] x86/insn: Support both signed 32-bit and 64-bit effective addresses

by Ricardo Neri

On Tue, 2017-04-25 at 15:51 +0200, Borislav Petkov wrote: > On Tue, Mar 07, 2017 at 04:32:45PM -0800, Ricardo Neri wrote: > > The 32-bit and 64-bit address encodings are identical. This means that we > > can use the same function in both cases. In order to reuse the function for > > 32-bit address encodings, we must sign-extend our 32-bit signed operands to > > 64-bit signed variables (only for 64-bit builds). To decide on whether sign > > extension is needed, we rely on the address size as given by the > > instruction structure. > > > > Lastly, before computing the linear address, we must truncate our signed > > 64-bit signed effective address if the address size is 32-bit. > > > > Cc: Dave Hansen <dave.hansen(a)linux.intel.com> > > Cc: Adam Buchbinder <adam.buchbinder(a)gmail.com> > > Cc: Colin Ian King <colin.king(a)canonical.com> > > Cc: Lorenzo Stoakes <lstoakes(a)gmail.com> > > Cc: Qiaowei Ren <qiaowei.ren(a)intel.com> > > Cc: Arnaldo Carvalho de Melo <acme(a)redhat.com> > > Cc: Masami Hiramatsu <mhiramat(a)kernel.org> > > Cc: Adrian Hunter <adrian.hunter(a)intel.com> > > Cc: Kees Cook <keescook(a)chromium.org> > > Cc: Thomas Garnier <thgarnie(a)google.com> > > Cc: Peter Zijlstra <peterz(a)infradead.org> > > Cc: Borislav Petkov <bp(a)suse.de> > > Cc: Dmitry Vyukov <dvyukov(a)google.com> > > Cc: Ravi V. Shankar <ravi.v.shankar(a)intel.com> > > Cc: x86(a)kernel.org > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon(a)linux.intel.com> > > --- > > arch/x86/lib/insn-eval.c | 44 ++++++++++++++++++++++++++++++++------------ > > 1 file changed, 32 insertions(+), 12 deletions(-) > > > > diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c > > index edb360f..a9a1704 100644 > > --- a/arch/x86/lib/insn-eval.c > > +++ b/arch/x86/lib/insn-eval.c > > @@ -559,6 +559,15 @@ int insn_get_reg_offset_sib_index(struct insn *insn, struct pt_regs *regs) > > return get_reg_offset(insn, regs, REG_TYPE_INDEX); > > } > > > > +static inline long __to_signed_long(unsigned long val, int long_bytes) > > +{ > > +#ifdef CONFIG_X86_64 > > + return long_bytes == 4 ? (long)((int)((val) & 0xffffffff)) : (long)val; > > I don't think this always works as expected: > > --- > typedef unsigned int u32; > typedef unsigned long u64; > > int main() > { > u64 v = 0x1ffffffff; > > printf("v: %ld, 0x%lx, %ld\n", v, v, (long)((int)((v) & 0xffffffff))); > > return 0; > } > -- > ... > > v: 8589934591, 0x1ffffffff, -1 > > Now, this should not happen on 32-bit because unsigned long is 32-bit > there but can that happen on 64-bit? This is the reason I check the value of long_bytes. If long_bytes is not 4, being the only other possible value 8 (perhaps I need to issue an error when the value is not any of these values), the cast is simply (long)val. I modified your test program with: printf("v: %ld, 0x%lx, %ld, %ld\n", v, v, (long)((int)((v) & 0xffffffff)), (long)v); and I get: v: 8589934591, 0x1ffffffff, -1, 8589934591. Am I missing something? > > > +#else > > + return (long)val; > > +#endif > > +} > > + > > /* > > * return the address being referenced be instruction > > * for rm=3 returning the content of the rm reg > > @@ -567,19 +576,21 @@ int insn_get_reg_offset_sib_index(struct insn *insn, struct pt_regs *regs) > > void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs) > > { > > unsigned long linear_addr, seg_base_addr; > > - long eff_addr, base, indx; > > - int addr_offset, base_offset, indx_offset; > > + long eff_addr, base, indx, tmp; > > + int addr_offset, base_offset, indx_offset, addr_bytes; > > insn_byte_t sib; > > > > insn_get_modrm(insn); > > insn_get_sib(insn); > > sib = insn->sib.value; > > + addr_bytes = insn->addr_bytes; > > > > if (X86_MODRM_MOD(insn->modrm.value) == 3) { > > addr_offset = get_reg_offset(insn, regs, REG_TYPE_RM); > > if (addr_offset < 0) > > goto out_err; > > - eff_addr = regs_get_register(regs, addr_offset); > > + tmp = regs_get_register(regs, addr_offset); > > + eff_addr = __to_signed_long(tmp, addr_bytes); > > This repeats throughout the function so it begs to be a separate: > > get_mem_addr() > > or so. Yes, the same pattern is used in all places except when using register operands (ModRM.rm == 11b). I will look into putting it in a function. > > > seg_base_addr = insn_get_seg_base(regs, insn, addr_offset, > > false); > > } else { > > @@ -591,20 +602,24 @@ void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs) > > * in the address computation. > > */ > > base_offset = get_reg_offset(insn, regs, REG_TYPE_BASE); > > - if (unlikely(base_offset == -EDOM)) > > + if (unlikely(base_offset == -EDOM)) { > > base = 0; > > - else if (unlikely(base_offset < 0)) > > + } else if (unlikely(base_offset < 0)) { > > goto out_err; > > - else > > - base = regs_get_register(regs, base_offset); > > + } else { > > + tmp = regs_get_register(regs, base_offset); > > + base = __to_signed_long(tmp, addr_bytes); > > + } > > > > indx_offset = get_reg_offset(insn, regs, REG_TYPE_INDEX); > > - if (unlikely(indx_offset == -EDOM)) > > + if (unlikely(indx_offset == -EDOM)) { > > indx = 0; > > - else if (unlikely(indx_offset < 0)) > > + } else if (unlikely(indx_offset < 0)) { > > goto out_err; > > - else > > - indx = regs_get_register(regs, indx_offset); > > + } else { > > + tmp = regs_get_register(regs, indx_offset); > > + indx = __to_signed_long(tmp, addr_bytes); > > + } > > > > eff_addr = base + indx * (1 << X86_SIB_SCALE(sib)); > > seg_base_addr = insn_get_seg_base(regs, insn, > > @@ -625,13 +640,18 @@ void __user *insn_get_addr_ref(struct insn *insn, struct pt_regs *regs) > > } else if (addr_offset < 0) { > > goto out_err; > > } else { > > - eff_addr = regs_get_register(regs, addr_offset); > > + tmp = regs_get_register(regs, addr_offset); > > + eff_addr = __to_signed_long(tmp, addr_bytes); > > } > > seg_base_addr = insn_get_seg_base(regs, insn, > > addr_offset, false); > > } > > eff_addr += insn->displacement.value; > > } > > + /* truncate to 4 bytes for 32-bit effective addresses */ > > + if (addr_bytes == 4) > > + eff_addr &= 0xffffffff; > > Why again? eff_addr is a long variable, which in x86_64 has 64-bit. However, in 32-bit segments the effective address is 32-bit. Thus, I discard the 32 most significant bytes. Thanks and BR, Ricardo

April 27, 2017