February 2016 - Wine-devel - list.winehq.org

Re: [PATCH] advapi32/tests: Simplify tests using todo_wine_if()
by Marvin 11 Feb '16

11 Feb '16

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=20371 Your paranoid android. === w1064 (32 bit security) === security.c:3753: Test failed: MACHINE\Software owner SID (S-1-5-18) != Administrators SID (S-1-5-32-544). security.c:3794: Test failed: Builtin Users ACE has unexpected flags (0x12 != 0xa) security.c:3798: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:3807: Test failed: Builtin Admins ACE has unexpected flags (0x12 != 0x0)

1 0

GSoC Ideas page
by André Hentschel 11 Feb '16

11 Feb '16

Hi, please update https://wiki.winehq.org/Summer_Of_Code with your Ideas and mention yourself as possible mentor where appropriate. Some things can be dropped I think, OpenMP maybe? Thanks!

5 5

Re: [PATCH 1/2] wpcap: Implement pcap_dump_open and pcap_dump(try 2)
by Sebastian Lackner 11 Feb '16

11 Feb '16

I'm not the maintainer of wpcap, nevertheless I hope its fine to give some suggestions how to improve the code. On 11.02.2016 09:55, Jianqiu Zhang wrote: > From 770a1250794833aa7d9a9596a7fc56bfc19c65f0 Mon Sep 17 00:00:00 2001 > From: Jianqiu Zhang <zhangjianqiu_133(a)yeah.net> > Date: Thu, 29 Oct 2015 15:33:28 +0800 > Subject: [PATCH 1/2] wpcap: Implement pcap_dump_open and pcap_dump > > Signed-off-by: Jianqiu Zhang <zhangjianqiu_133(a)yeah.net> > --- > dlls/wpcap/wpcap.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++ > dlls/wpcap/wpcap.spec | 4 ++-- > 2 files changed, 56 insertions(+), 2 deletions(-) > > diff --git a/dlls/wpcap/wpcap.c b/dlls/wpcap/wpcap.c > index ae9e482..e03e18e 100644 > --- a/dlls/wpcap/wpcap.c > +++ b/dlls/wpcap/wpcap.c > @@ -22,6 +22,8 @@ > #include "winsock2.h" > #include "windef.h" > #include "winbase.h" > +#include "winternl.h" > +#include "ntstatus.h" > #include "wine/debug.h" > > WINE_DEFAULT_DEBUG_CHANNEL(wpcap); > @@ -40,6 +42,8 @@ WINE_DECLARE_DEBUG_CHANNEL(winediag); > #define PCAP_SRC_IFLOCAL 3 > #endif > > +#define MAX_LENGTH 1024 In this case copying is not necessary at all. In other cases you would want to use MAX_PATH instead of own definitions. > + > void CDECL wine_pcap_breakloop(pcap_t *p) > { > TRACE("(%p)\n", p); > @@ -339,3 +343,53 @@ int CDECL wine_wsockinit(void) > if (WSAStartup(MAKEWORD(1,1), &wsadata)) return -1; > return 0; > } > + > +pcap_dumper_t* CDECL wine_pcap_dump_open(pcap_t *p, const char *fname) > +{ > + UNICODE_STRING nt_name, dospathW; > + ANSI_STRING fname_dos; > + ANSI_STRING fname_unix; > + CHAR fpath[MAX_LENGTH] = { 0 }; You don't need this char array (see comment below). > + NTSTATUS res; Not really critical, but "status" is often a bit more meaningful. > + TRACE("(%p %s)\n", p, fname); > + RtlInitAnsiString(&fname_dos, fname); > + res = RtlAnsiStringToUnicodeString(&dospathW, &fname_dos, TRUE); > + if(res) Minor issue, but the coding style in this file seems to be with a space between "if" and "(". > + { > + SetLastError(RtlNtStatusToDosError(res)); > + return NULL; > + } > + if(!RtlDosPathNameToNtPathName_U(dospathW.Buffer, &nt_name, NULL, NULL)) > + { > + RtlFreeUnicodeString(&dospathW); > + SetLastError(ERROR_FILENAME_EXCED_RANGE); Basically all places in wine use ERROR_PATH_NOT_FOUND in this case. > + return NULL; > + } > + res = wine_nt_to_unix_file_name(&nt_name, &fname_unix, FILE_OPEN_IF, FALSE); > + if(res) > + { > + if(res == STATUS_NO_SUCH_FILE) > + SetLastError(ERROR_SUCCESS); Setting error code in case of success is not useful. > + else > + { > + RtlFreeUnicodeString(&dospathW); > + RtlFreeUnicodeString(&nt_name); > + SetLastError(RtlNtStatusToDosError(res)); > + return NULL; > + } > + } > + else > + { > + SetLastError(ERROR_FILE_EXISTS); Similar to above, we don't care about the error code in case of success. > + } > + memcpy(fpath, fname_unix.Buffer, fname_unix.Length); > + RtlFreeUnicodeString(&nt_name); > + RtlFreeUnicodeString(&dospathW); > + RtlFreeAnsiString(&fname_unix); > + return pcap_dump_open(p, fpath); Instead of copying the string, please store the return value of pcap_dump_open in a variable. > +} > + > +void CDECL wine_pcap_dump(u_char *user, const struct pcap_pkthdr *h, const u_char *sp) > +{ > + return pcap_dump(user, h, sp); > +} > diff --git a/dlls/wpcap/wpcap.spec b/dlls/wpcap/wpcap.spec > index 66303b4..541fc49 100644 > --- a/dlls/wpcap/wpcap.spec > +++ b/dlls/wpcap/wpcap.spec > @@ -16,12 +16,12 @@ > @ cdecl pcap_datalink_val_to_description(long) wine_pcap_datalink_val_to_description > @ cdecl pcap_datalink_val_to_name(long) wine_pcap_datalink_val_to_name > @ cdecl pcap_dispatch(ptr long ptr ptr) wine_pcap_dispatch > -@ stub pcap_dump > +@ cdecl pcap_dump(ptr ptr ptr) wine_pcap_dump > @ stub pcap_dump_close > @ stub pcap_dump_file > @ stub pcap_dump_flush > @ stub pcap_dump_ftell > -@ stub pcap_dump_open > +@ cdecl pcap_dump_open(ptr str) wine_pcap_dump_open > @ stub pcap_file > @ stub pcap_fileno > @ cdecl pcap_findalldevs(ptr ptr) wine_pcap_findalldevs > > > >

1 0

Re: [v2 PATCH] user32/text: Show Color message isn't send during startup
by Alexandre Julliard 11 Feb '16

11 Feb '16

Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com> writes: > @@ -214,6 +218,12 @@ static INT_PTR CALLBACK edit_dialog_proc(HWND hdlg, UINT msg, WPARAM wparam, LPA > PostMessageA(hdlg, WM_USER, 0xdeadbeef, 2); > break; > > + /* Set Text During startup */ > + case 11: > + SendMessageA(hok, WM_SETTEXT, 0, (WPARAM)"Testing"); > + PostMessageA(hdlg, WM_USER, 0xdeadbeef, 2); > + break; Why are you setting the text on a button, and not touching the edit control? -- Alexandre Julliard julliard(a)winehq.org

1 0

Re: [PATCH 2/3] msvcp110: Add tr2_sys__Symlink implementation and test.
by Piotr Caban 11 Feb '16

11 Feb '16

On 01/11/16 14:56, YongHao Hu wrote: > + delete_flag = 1; > + for(i=0; i<sizeof(tests)/sizeof(tests[0]); i++) { > + errno = 0xdeadbeef; > + SetLastError(0xdeadbeef); > + ret = p_tr2_sys__Symlink(tests[i].existing_path, tests[i].new_path); > + if(ret==ERROR_PRIVILEGE_NOT_HELD || ret==ERROR_INVALID_FUNCTION) { > + delete_flag = 0; > + tests[i].last_error = ret; > + win_skip("Privilege not held or symbolic link not supported, skipping symbolic link tests.\n"); I think it would be nicer if the tests are really skipped in this case. Please detect such case and return from the function instead of adding workarounds in the code for it. Thanks, Piotr

2 3

Re: [PATCH 1/2] wpcap: Implement pcap_dump_open and pcap_dump
by André Hentschel 10 Feb '16

10 Feb '16

Hi, Sorry for the delay, but I haven't noticed that I got assigned for reviewing. Could you please tell me the name of the application so I can do some testing? Further reviewing inlined below: Am 07.01.2016 um 09:49 schrieb Jianqiu Zhang: > + > +pcap_dumper_t* CDECL wine_pcap_dump_open(pcap_t *p, const char *fname) > +{ > + return pcap_dump_open(p, fname); > +} This can't work, the wpcap function might be called with fname set to "C:\dump.pcap" and the native function will be confused by that. > +void CDECL wine_pcap_dump(u_char *user, const struct pcap_pkthdr *h, const u_char *sp) > +{ > + return pcap_dump(user, h, sp); > +} This should work, but where are pcap_dump_close and friends? > diff --git a/dlls/wpcap/wpcap.spec b/dlls/wpcap/wpcap.spec > index 0e1e208..d7efd5e 100644 > --- a/dlls/wpcap/wpcap.spec > +++ b/dlls/wpcap/wpcap.spec > @@ -16,12 +16,12 @@ > @ cdecl pcap_datalink_val_to_description(long) wine_pcap_datalink_val_to_description > @ cdecl pcap_datalink_val_to_name(long) wine_pcap_datalink_val_to_name > @ cdecl pcap_dispatch(ptr long ptr ptr) wine_pcap_dispatch > -@ stub pcap_dump > +@ cdecl pcap_dump(str ptr str) wine_pcap_dump Should be (ptr ptr ptr), there are no strings to be displayed

3 9

Re: [PATCH] user32/tests: Simplify some tests using todo_wine_if()
by Marvin 10 Feb '16

10 Feb '16

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=20354 Your paranoid android. === w8 (32 bit sysparams) === sysparams.c:2488: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2499: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w864 (32 bit sysparams) === sysparams.c:2488: Test failed: Waiting for the WM_DISPLAYCHANGE message timed out sysparams.c:2499: Test failed: Set bpp 32, but WM_DISPLAYCHANGE reported bpp -1 === w1064 (32 bit monitor) === monitor.c:248: Test failed: Unexpected ChangeDisplaySettingsEx() return code for resolution[0]: -2 monitor.c:248: Test failed: Unexpected ChangeDisplaySettingsEx() return code for resolution[2]: -2 === w1064 (32 bit msg) === msg.c:6290: Test failed: Update region shouldn't be empty msg.c:6524: Test failed: InvalidateErase: 0: the msg sequence is not complete: expected 0085 - actual 0000 msg.c:6526: Test failed: Paint: 0: the msg sequence is not complete: expected 000f - actual 0000 msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different msg.c:6296: Test failed: Regions are different === w2008s64 (32 bit win) === win.c:8627: Test failed: Expected foreground window 00020078, got 00030040

1 0

Re: [PATCH] gdi32/tests: Simplify tests using todo_wine_if()
by Marvin 10 Feb '16

10 Feb '16

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=20353 Your paranoid android. === wvistau64 (32 bit font) === Failure running script in VM: network read timed out (Connect:AgentVersion.h:0/9)

1 0

Re: [3/3] advapi32/tests: Add tests for Win8 pseudo handles.
by Marvin 10 Feb '16

10 Feb '16

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=20338 Your paranoid android. === w1064 (32 bit security) === security.c:3774: Test failed: MACHINE\Software owner SID (S-1-5-18) != Administrators SID (S-1-5-32-544). security.c:3815: Test failed: Builtin Users ACE has unexpected flags (0x12 != 0xa) security.c:3819: Test failed: Builtin Users ACE has unexpected mask (0x20019 != 0x80000000) security.c:3828: Test failed: Builtin Admins ACE has unexpected flags (0x12 != 0x0)

2 1

Re: [PATCH 5/7] wined3d: Copy channel info for typed formats from typeless formats.
by Henri Verbeet 10 Feb '16

10 Feb '16

On 10 February 2016 at 02:14, Józef Kucia <jkucia(a)codeweavers.com> wrote: > @@ -1436,8 +1408,7 @@ static inline int getFmtIdx(enum wined3d_format_id format_id) > { > /* First check if the format is at the position of its value. > * This will catch the argb formats before the loop is entered. */ > - if (format_id < (sizeof(formats) / sizeof(*formats)) > - && formats[format_id].id == format_id) > + if (format_id < ARRAY_SIZE(formats) && formats[format_id].id == format_id) > { > return format_id; > } > @@ -1445,10 +1416,10 @@ static inline int getFmtIdx(enum wined3d_format_id format_id) > { > unsigned int i; > > - for (i = 0; i < (sizeof(formats) / sizeof(*formats)); ++i) > - { > + for (i = 0; i < ARRAY_SIZE(formats); ++i) > if (formats[i].id == format_id) return i; > - } > + for (i = 0; i < ARRAY_SIZE(typed_formats); ++i) > + if (typed_formats[i].id == format_id) return ARRAY_SIZE(formats) + i; > } > return -1; > } I'm not sure this is really the way to go. I'd prefer to just reorder things in such a way that the format id is the index in all cases except for FourCC formats. Patch 7/7 goes a long way in that direction, but to me it looks more complicated than just having a small table to remap the FourCC formats to an index. One additional advantage would be that we could then also remap e.g. DXT2 and DXT3 to BC2. E.g.: struct { enum wined3d_format_id id; unsigned int idx; } remap[] = { {UYVY, FOURCC_BASE} {YUY2, FOURCC_BASE + 1} {YV12, FOURCC_BASE + 2} {DXT1, BC1}, {DXT2, BC2}, {DXT3, BC2}, {DXT4, BC3}, {DXT5, BC3}, };

2 2