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

Re: [PATCH] wmvcore/tests: add tests for WMReader methods

by Jefferson Carpenter

Thanks for the feedback. I am on it. Couple questions: Why not use Sleep? (Some other tests such as ntdll/tests/threadpool.c seem to be using it). And why not use malloc / free? On Mon, Jul 24, 2017 at 4:47 AM, Alistair Leslie-Hughes < leslie_alistair(a)hotmail.com> wrote: > Hi Jefferson, > > On 24/07/17 08:26, Jefferson Carpenter wrote: > > Create a sample WMA file, which is saved into a temp directory at the > > beginning of the wmvcore tests. > > > > Added a IWMReaderCallback implementation for use in conformance tests. > > > > Tested various playback-related WMReader methods. > > > > Changes in version 2 of this patch: Declared loop variable at top of > > function, and using C89-style comments. > > The callback interface can just be static, see mshtml/tests/htmldoc.c > for an example. Maybe using the CHECKED_CALLED syntax to check if > something was been called. If the order of called is important then see > msxml3/tests/saxreader.c for an example. > > You cannot use Sleep in tests, no matter how small it has to wait, > instead use an Event+WaitForSingleObject. > > Use HeapAlloc/HeapFree instead of malloc/free, create a worker function > if required. > > > + IWMReader *reader; > Leaking reader > > + /* Get output props. */ > + hr = IWMReader_GetOutputProps(reader, 0, &props); > + todo_wine ok(hr == S_OK, "Failed to get output props: 0x%08x\n", > Leaking props here. > > .... > + for (i = 0; i < formatCount; i++) { > + /* Get output props for format. */ > + hr = IWMReader_GetOutputFormat(reader, 0, i, &props); > + todo_wine ok(hr == S_OK, "Failed to get output format: > and here. > > > Best Regards > Alistair Leslie-Hughes >

July 24, 2017

Re: mscoree: Update for mono profiler api v2.

by Vincent Povirk

Please don't commit this. They just changed it again and put in compatibility functions for the old API.

July 24, 2017

Re: Some clarification of msi messages for translation and pedantic remarks

by Zebediah Figura

On 07/24/2017 10:18 AM, Julian Rüger wrote: > (snip) I used a test program that registered a record callback to retrieve the description and template of every action we have implemented in Wine, so as near as I can tell these are exact, duplicate spaces and all. I don't honestly know myself what some of these actions do. We could certainly provide more descriptive strings for them in Wine (and fix the duplicate spaces, etc) since I doubt any application is going to depend on them.

July 24, 2017

Some clarification of msi messages for translation and pedantic remarks

by Julian Rüger

Hi Zebediah (and list), I'd like some clarification of a few terms used in the new msi messages, to make sure I translate the correct meaning. :) #: msi.rc:103 msgid "Searching for related applications" ... #: msi.rc:111 msgid "Migrating feature states from related applications" related as in 'similar' or 'belonging together' or something else? I'd guess #2. #: msi.rc:113 msgid "Publishing assembly information" assembly as in 'binary/executable file/dll', 'the assembler language' or just 'putting together'? I strongly suspect #1, and that is a tough one. There is no fitting translation in German and this is not one of the many, many cases where you would just use the original English term. It sounds totally awkward. (Maybe because words ending in y are usually Germanized to use other suffixes. Just does not work with 'Assembly') EDIT: Never mind, I just looked it up and M$ does indeed use the untranslated original, even in compound words.. Ughhh! #: msi.rc:120 msgid "Registering Class servers" ... #: msi.rc:122 msgid "Registering extension servers" It's 'Server' in German as well for the hardware (machine/host) or applications/services like Apache, but this feels like a different thing. Am I being silly? Maybe this can only be answered by other German speakers. #: msi.rc:130 msi.rc:154 msgid "Updating environment strings" Can I just call this environment variables? Sounds more natural than the compound word with string ("Umgebungszeichenketten")... Thanks for your help and for all your work! Best, Julian PS: Some other questions/comments, unrelated to translation: #: msi.rc:168 msi.rc:171 msi.rc:175 msgid "File: [1], Directory: [9], Size: [6]" ... #: msi.rc:170 msgid "File: [1], Directory: [9], Size: [6]" Are these supposed to be separate, but identical? #: msi.rc:173 msgid "File: [1], Dependencies: [2]" ^^ Two spaces on purpose? #: msi.rc:176 msi.rc:177 msgid "Application Context:[1], Assembly Name:[2]" Missing spaces? #: msi.rc:189 msi.rc:209 msgid "Name: [1], Value: [2], Action [3]" Missing colon? (Action: [3])? #: msi.rc:178 msgid "File: [1], Directory: [2], Size: [3]" ... #: msi.rc:193 msi.rc:210 msgid "File: [1], Section: [2], Key: [3], Value: [4]" Duplicate spaces again?

July 24, 2017

Re: [PATCH 4/5] reg: Use a helper function to allocate and re-allocate memory

by Alexandre Julliard

Hugh McMaster <hugh.mcmaster(a)outlook.com> writes: > @@ -174,6 +177,26 @@ static BOOL ask_confirm(unsigned int msgid, WCHAR *reg_info) > } > } > > +#define CHECK_ENOUGH_MEMORY(p) \ > +if (!(p)) \ > +{ \ > + output_message(STRING_OUT_OF_MEMORY, __FILE__, __LINE__); \ > + exit(1); \ > +} > + > +static void *resize_buffer(void *buf, size_t count) > +{ > + void *new_buf; > + > + if (buf) > + new_buf = HeapReAlloc(GetProcessHeap(), 0, buf, count); > + else > + new_buf = HeapAlloc(GetProcessHeap(), 0, count); > + > + CHECK_ENOUGH_MEMORY(new_buf); > + return new_buf; > +} The macro is not needed, and the message doesn't need to be localized, since this would require allocating memory. I think you'll also want a separate wrapper for HeapAlloc. -- Alexandre Julliard julliard(a)winehq.org

July 24, 2017

Re: [PATCH 1/3] msvcrt: Don't set errno when calling trigonometric functions with NAN.

by Piotr Caban

On 07/24/17 07:40, Alex Henrie wrote: > The errno behavior is not relevant to the bugs I was trying to fix; it > was just something that I thought I could fix quickly and easily. Do > you think that it is worthwhile for me to continue working on it? I think that there's more value in fixing bugs that lets application run. On the other hand this code can use some cleanup. I'm just not sure how to review code that doesn't fix any real application, changes function behavior to be compatible with newer version of the dll while making it less compatible with older versions. I was hoping that there might be a solution that will preserve compatibility with older versions and improve compatibility with newer ones (not necessarily setting errno/calling _matherr in the same way as native does for all versions of the dll). If you're going to change most of this functions it's worth adding proper support for matherr function. Thanks, Piotr

July 24, 2017

Re: [1/2] gdi32/tests: Add test for SetPixel erase with R2_XORPEN

by Huw Davies

On Mon, Jul 24, 2017 at 12:57:30AM +1000, Tim Worthington wrote: > diff --git a/dlls/gdi32/tests/dib.c b/dlls/gdi32/tests/dib.c > --- a/dlls/gdi32/tests/dib.c > +++ b/dlls/gdi32/tests/dib.c > @@ -2703,6 +2703,26 @@ > ret = ExtFloodFill( hdc, -1, -1, RGB( 0, 0xff, 0 ), FLOODFILLSURFACE ); > ok (!ret, "got ret %d\n", ret); > > + /* SetPixel erase with R2_XORPEN */ > + if ( ( bmi->bmiHeader.biBitCount >= 16 ) > + || ( ( bmi->bmiHeader.biClrUsed == 256 ) && ( bmi->bmiHeader.biBitCount == 8 ) ) ) > + { > + int rop2; > + COLORREF set, get; > + > + rop2 = GetROP2( hdc ); > + for (i = 0; i < 256; i++) > + { > + SetROP2( hdc, R2_COPYPEN ); > + set = SetPixel( hdc, i * 2, 30, RGB( (i & 0x07) << 5, (i & 0x38) << 2, i & 0xc0 ) ); > + SetROP2( hdc, R2_XORPEN ); > + SetPixel( hdc, i * 2, 30, set ); > + get = GetPixel( hdc, i * 2, 30 ); > + ok( get == 0, "got %08x and %08x\n", set, get ); > + } > + SetROP2( hdc, rop2 ); > + } > + > SelectObject(hdc, orig_brush); > SelectObject(hdc, orig_pen); > DeleteObject(solid_brush); Hi Tim, Thank you for your contribution to Wine. There were a number of issues with this series, which by the time I'd reviewed them meant I'd essentially re-written the patches; so I just sent them in: https://source.winehq.org/patches/data/136093 https://source.winehq.org/patches/data/136094 This seemed more efficient than explaining the problems and suggesting alternative approaches. If you're interested, please free feel to look at my patches and ask if there's anything you're curious about. Huw.

July 24, 2017

Re: [PATCH] wmvcore/tests: add tests for WMReader methods

by Alistair Leslie-Hughes

Hi Jefferson, On 24/07/17 08:26, Jefferson Carpenter wrote: > Create a sample WMA file, which is saved into a temp directory at the > beginning of the wmvcore tests. > > Added a IWMReaderCallback implementation for use in conformance tests. > > Tested various playback-related WMReader methods. > > Changes in version 2 of this patch: Declared loop variable at top of > function, and using C89-style comments. The callback interface can just be static, see mshtml/tests/htmldoc.c for an example. Maybe using the CHECKED_CALLED syntax to check if something was been called. If the order of called is important then see msxml3/tests/saxreader.c for an example. You cannot use Sleep in tests, no matter how small it has to wait, instead use an Event+WaitForSingleObject. Use HeapAlloc/HeapFree instead of malloc/free, create a worker function if required. + IWMReader *reader; Leaking reader + /* Get output props. */ + hr = IWMReader_GetOutputProps(reader, 0, &props); + todo_wine ok(hr == S_OK, "Failed to get output props: 0x%08x\n", Leaking props here. .... + for (i = 0; i < formatCount; i++) { + /* Get output props for format. */ + hr = IWMReader_GetOutputFormat(reader, 0, i, &props); + todo_wine ok(hr == S_OK, "Failed to get output format: and here. Best Regards Alistair Leslie-Hughes

July 24, 2017

Re: ntdll: Add padding to the stack structure for quad-word alignment on ARM64

by Alexandre Julliard

André Hentschel <nerv(a)dawncrow.de> writes: > EXCEPTION_RECORD is not correctly aligned and until bd1f7ef1868dd563a82eacd1bf414bf8d1a7254e, CONTEXT wasn't either, > which ended up in a correctly aligned structure, but the mentioned commit changed that. > > Signed-off-by: André Hentschel <nerv(a)dawncrow.de> > --- > dlls/ntdll/signal_arm64.c | 1 + > 1 file changed, 1 insertion(+) You should probably handle that in the existing alignment computation instead. -- Alexandre Julliard julliard(a)winehq.org

July 24, 2017

Re: [PATCH v2 3/3] msvcr120: Add log1p.

by Alex Henrie

2017-07-17 8:18 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>: > Hi, > > On 07/16/17 05:11, Alex Henrie wrote: >> >> 2017-07-14 4:20 GMT-06:00 Piotr Caban <piotr.caban(a)gmail.com>: >>> >>> On 07/14/17 07:14, Alex Henrie wrote: >>>> >>>> >>>> +/********************************************************************* >>>> + * log1pf (MSVCR120.@) >>>> + */ >>>> +float CDECL MSVCR120_log1pf(float x) >>>> +{ >>>> +#ifdef HAVE_LOG1PF >>>> + return log1pf(x); >>>> +#else >>>> + return log(1 + x); >>>> +#endif >>>> +} >>> >>> >>> Please also set errno in this function. >> >> >> The Microsoft implementations of log2 and log1p are buggy: Despite >> what MSDN says, they do not set errno. >> >> In fact, as far as I can tell, Microsoft's acosh, atanh, and expm1 >> don't set errno either. Do you want tests for this? > > New versions are setting the errno. You can e.g. test it with ucrtbase. I don't know what I was doing before. All of these functions first appeared in msvcr120, and when I tested them again, both the msvcr120 and ucrtbase versions set errno correctly. -Alex

July 24, 2017