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

Re: configure: Set PKG_CONFIG_PATH to get the right settings for the target host.

by Michael Müller

Setting PKG_CONFIG_PATH doesn't seem like a good option for me. You would need to implement the path for every possible distribution and platform to not cause any regressions. On Arch Linux it is for example /usr/lib32/pkgconfig. On FreeBSD it is /usr/libdata/pkgconfig/. Not to speak of cross compilation cases in which users might have a special build of pkgconfig where overriding PKG_CONFIG_PATH doesn't make any sense. Regards, Michael

June 6, 2017

Re: [PATCH v2 1/2] msvcrt: Clean up registered C++ object in handler.

by Piotr Caban

Hi Daniel, On 06/06/17 00:43, Daniel Lehman wrote: > +typedef void (CALLBACK *__WINE_FINALLY_CTX)(BOOL, PEXCEPTION_RECORD, void*); The idea behind __FINALLY_CTX was to model native __finally behavior while extending its functionality for wine. It's not possible to call GetExceptionInfo/GetExceptionCode inside of finally on Windows so I don't think it's a change in good direction. If you really need the exception record you shouldn't use __FINALLY_CTX (but I don't think you need). > +static void CALLBACK cxx_catch_cleanup(BOOL normal, EXCEPTION_RECORD *rec, void *ctx) > +{ > + thread_data_t *data = msvcrt_get_thread_data(); > + cxx_frame_info *frame = ctx; > + > + if (normal) > + __CxxUnregisterExceptionObject(frame, FALSE); > + else > + { > + if (cxx_is_consolidate(rec)) > + rec = (void*)rec->ExceptionInformation[4]; > + > + __CxxUnregisterExceptionObject(frame, > + rec->ExceptionCode == CXX_EXCEPTION && > + data->exc_record->ExceptionCode == CXX_EXCEPTION && > + rec->ExceptionInformation[1] == data->exc_record->ExceptionInformation[1]); > + } Do you need to call __CxxUnregisterExceptionObject with TRUE as second argument in any case except of rethrow? > /* FIXME: native does local_unwind here in case of exception rethrow */ If you only need to handle rethrow in a special way I think it should be done differently. Something like this should do the job: __TRY { __TRY { ret_addr = handler(0, frame); } __EXCEPT(rethrow_filter) { /* probably all of this should be done inside of the filter */ /* do some unwinding here or add a FIXME */ catch_cleanup_ctx.in_use = TRUE; rethrow; } __ENDTRY; } __FINALLY_CTX(cxx_catch_cleanup, &catch_cleanup_ctx); The above code is incorrect if exception object needs to be preserved not only in case of rethrow. What do you think about it? Thanks, Piotr

June 6, 2017

configure: Set PKG_CONFIG_PATH to get the right settings for the target host.

by Francois Gouget

As pointed out by Henri we should not override PKG_CONFIG_PATH in case it's already set. Here we still override it if it is set to an empty string but that can be worked around by setting it to a nonexistent path or maybe even a single colon. Ideally we would use the right <triplet>-pkg-config tool for the target host. However Red Hat and SUSE respectively add '-redhat' and '-suse' to the triplet which makes it hard to find the right tool. Furthermore on Debian and derivatives a lot of development packages do not support multiarch so that the required .pc files would not be found. Setting PKG_CONFIG_PATH side-steps the non-standard Red Hat and SUSE triplets. It also lets pkg-config fall back to the 'wrong' .pc files on Debian which is fine in most cases, which using the right one in cases where it matters (GStreamer 1.0), assuming they are actually present. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- configure.ac | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/configure.ac b/configure.ac index 71bf7ca2bf7..b7e4eff3b11 100644 --- a/configure.ac +++ b/configure.ac @@ -151,6 +151,11 @@ case $host in host_cpu="i386" notice_platform="32-bit " AC_SUBST(TARGETFLAGS,"-m32") + if test "x$PKG_CONFIG_PATH" = "x" + then + PKG_CONFIG_PATH="/usr/lib/i386-linux-gnu/pkgconfig:/usr/lib/pkgconfig" + export PKG_CONFIG_PATH + fi enable_win16=${enable_win16:-yes} else if test "x${GCC}" = "xyes" @@ -166,6 +171,11 @@ case $host in host_cpu="x86_64" notice_platform="64-bit " AC_SUBST(TARGETFLAGS,"-m64") + if test "x$PKG_CONFIG_PATH" = "x" + then + PKG_CONFIG_PATH="/usr/lib/x86_64-linux-gnu/pkgconfig:/usr/lib64/pkgconfig" + export PKG_CONFIG_PATH + fi fi ;; arm*) -- 2.11.0

June 6, 2017

Re: [PATCH v7 09/26] x86/insn-eval: Add utility function to identify string instructions

by Borislav Petkov

On Mon, Jun 05, 2017 at 11:01:21PM -0700, Ricardo Neri wrote: > If I was to leave out string instructions from this function it should > be renamed as is_string_instruction_non_lods_outs. In my opinion this > separation makes the code more clear and I would end up having logic to > decide which segment register to use in two places. Does it makes sense > to you? Ok, sure. Thanks. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

June 6, 2017

Re: [PATCH v7 07/26] x86/insn-eval: Do not BUG on invalid register type

by Borislav Petkov

On Mon, Jun 05, 2017 at 11:06:58PM -0700, Ricardo Neri wrote: > I agree that insn-eval reads somewhat funny. I did not want to go with > insn-dec.c as insn.c, in my opinion, already decodes the instruction > (i.e., it finds prefixes, opcodes, ModRM, SIB and displacement bytes). > In insn-eval.c I simply take those decoded parameters and evaluate them > to obtain the values they contain (e.g., a specific memory location). > Perhaps, insn-resolve.c could be a better name? Or maybe isnn-operands? So actually I'm gravitating towards calling all that instruction "massaging" code with a single prefix to denote this comes from the insn decoder/handler/whatever... I.e., "insn-decoder: x86: invalid register type" or "inat: x86: invalid register type" or something to that effect. I mean, If we're going to grow our own - as we do, apparently - maybe it all should be a separate entity with its proper name. Hmm. -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --

June 6, 2017

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

by Jacek Caban

On 05.06.2017 17:28, Henri Verbeet wrote: > On 3 June 2017 at 03:13, Alistair Leslie-Hughes > <leslie_alistair(a)hotmail.com> wrote: >> 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". >> > That's a rather poor justification for using typedefs, but > unfortunately also a really common one. What you're essentially doing > is obfuscating the fact that something is a > structure/enum/integer/pointer/etc., only to avoid a few keystrokes. > One of the reasons that's odd is that the time spent typing in the > code should be small compared to the time spent thinking about the > problem you're trying to solve. That justification is not really strong either. I don't see how using them makes code you're commenting any less readable. When it comes to personal preferences, in case of COM objects, the object name is often a public typedef from coclass declaration (although not in case of this particular type). Although there is no technical reason to use different declarations, I'd argue that it's the most intuitive choice. That said, there are different preferences. Alistair chose a declaration consistent with existing code within DLL, so I'd say he did the right choice. Jacek

June 6, 2017

Re: [v3 PATCH] oleaut32: Improve support for IDispatch in VarCat

by Jacek Caban

Hi Alistair, The patch looks mostly good now. On 26.05.2017 07:13, Alistair Leslie-Hughes wrote: > + hres = VariantChangeTypeEx(&bstrvar_left,tmp,0,VARIANT_ALPHABOOL|VARIANT_LOCALBOOL,VT_BSTR); > + if (hres == DISP_E_TYPEMISMATCH) > { It seems that might be the right thing to do, but one more test would make sure about that. Please add a test that will hit this case without VT_DISPATCH nor VT_NULL, since both of them are kind of special. As far as I can see, a VT_BYREF with NULL reference would be good for that. > - hres = VariantChangeTypeEx(&bstrvar_left,left,0,VARIANT_ALPHABOOL|VARIANT_LOCALBOOL,VT_BSTR); > - if (hres != S_OK) { > - VariantClear(&bstrvar_left); > - VariantClear(&bstrvar_right); > - return hres; > - } > + V_VT(&bstrvar_left) = VT_BSTR; > + V_BSTR(&bstrvar_left) = SysAllocString(sz_empty); You could just set it to NULL. Thanks, Jacek

June 6, 2017

configure: Set PKG_CONFIG_PATH to get the right settings for the target host.

by Francois Gouget

In an ideal world I think the previous patch would have been ok but due to the issues mentioned in the first paragraph this one may be better. Regarding the 64 bit case, since we feel it necessary to set CC="$CC -m64", I think we should also set PKG_CONFIG_PATH to make sure the 64 bit .pc files will be used rather than the 32 bit ones. What's a bit strange is that this branch does not have the "$cross_compiling" != "yes" check. This case being much less common I would be ok with dropping that part. Ideally we would use the right <triplet>-pkg-config tool for the target host. However Red Hat and SUSE respectively add '-redhat' and '-suse' to the triplet which makes it hard to find the right tool. Furthermore on Debian and derivatives a lot of development packages do not support multiarch so that the required .pc files would not be found. Setting PKG_CONFIG_PATH side-steps the non-standard Red Hat and SUSE triplets. It also lets pkg-config fall back to the 'wrong' .pc files on Debian which is fine in most cases, which using the right one in cases where it matters (GStreamer 1.0), assuming they are actually present. Signed-off-by: Francois Gouget <fgouget(a)codeweavers.com> --- configure.ac | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/configure.ac b/configure.ac index 71bf7ca2bf7..7ecf41846c5 100644 --- a/configure.ac +++ b/configure.ac @@ -151,6 +151,8 @@ case $host in host_cpu="i386" notice_platform="32-bit " AC_SUBST(TARGETFLAGS,"-m32") + PKG_CONFIG_PATH="/usr/lib/i386-linux-gnu/pkgconfig:/usr/lib/pkgconfig" + export PKG_CONFIG_PATH enable_win16=${enable_win16:-yes} else if test "x${GCC}" = "xyes" @@ -166,6 +168,8 @@ case $host in host_cpu="x86_64" notice_platform="64-bit " AC_SUBST(TARGETFLAGS,"-m64") + PKG_CONFIG_PATH="/usr/lib/x86_64-linux-gnu/pkgconfig:/usr/lib64/pkgconfig" + export PKG_CONFIG_PATH fi ;; arm*) -- 2.11.0

June 6, 2017

Re: [PATCH 3/3] inetcomm: Implemented IMimeBody::GetClassID()

by Jacek Caban

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

June 6, 2017

Re: [PATCH 2/3] inetcomm: Release previously set data (Valgrind)

by Jacek Caban

Signed-off-by: Jacek Caban <jacek(a)codeweavers.com>

June 6, 2017