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

Re: [PATCH] storage.dll16: Set OpenStorage/OpenStream output to NULL on failure.
by Nikolay Sivov 12 Feb '17

12 Feb '17

On 12.02.2017 2:40, Zebediah Figura wrote: > As per MSDN: https://msdn.microsoft.com/en-us/library/windows/desktop/aa380025(v=vs.85).… > This page covers Win32 variant, it doesn't necessary match how Win16 works, or Win32 too for that matter.

2 1

Re: [PATCH 1/3] storage.dll16: Fix negation in IStream::Seek.
by Alex Henrie 11 Feb '17

11 Feb '17

@@ -1196,9 +1196,8 @@ HRESULT CDECL IStream16_fnSeek(IStream16 *iface, LARGE_INTEGER offset, DWORD whe break; case SEEK_CUR: if (offset.u.HighPart < 0) { - /* FIXME: is this negation correct ? */ - offset.u.HighPart = -offset.u.HighPart; - offset.u.LowPart = (0xffffffff ^ offset.u.LowPart)+1; + offset.u.HighPart = ~offset.u.HighPart; + offset.u.LowPart = -offset.u.LowPart; assert(offset.u.HighPart==0); assert(This->offset.u.LowPart >= offset.u.LowPart); You're mixing tabs and spaces. -Alex

1 0

Re: [v4 2/3] ntdll: Implement FileIdInformation class support in NtQueryInformationFile (resend).
by Sebastian Lackner 10 Feb '17

10 Feb '17

On 07.02.2017 16:11, Jonathan Doron wrote: > Signed-off-by: Jonathan Doron <jond(a)wizery.com> > --- > configure.ac | 2 +- > dlls/ntdll/file.c | 28 ++++++++++++++++++++++- > dlls/ntdll/tests/file.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ > include/winternl.h | 9 ++++++++ > 4 files changed, 97 insertions(+), 2 deletions(-) > > diff --git a/configure.ac b/configure.ac > index c6d63b2..f66bf44 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -2262,7 +2262,7 @@ fi > dnl **** FIXME: what about mixed cases, where we need two of them? *** > > dnl Check for statfs members > -AC_CHECK_MEMBERS([struct statfs.f_bfree, struct statfs.f_bavail, struct statfs.f_frsize, struct statfs.f_ffree, struct statfs.f_favail, struct statfs.f_namelen],,, > +AC_CHECK_MEMBERS([struct statfs.f_bfree, struct statfs.f_bavail, struct statfs.f_frsize, struct statfs.f_ffree, struct statfs.f_favail, struct statfs.f_namelen, struct statfs.f_fsid.__val],,, > [#include <sys/types.h> > #ifdef HAVE_SYS_PARAM_H > # include <sys/param.h> > diff --git a/dlls/ntdll/file.c b/dlls/ntdll/file.c > index a6c1098..c6ff371 100644 > --- a/dlls/ntdll/file.c > +++ b/dlls/ntdll/file.c > @@ -2377,7 +2377,7 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, > 0, /* FileRenameInformationBypassAccessCheck */ > 0, /* FileLinkInformationBypassAccessCheck */ > 0, /* FileVolumeNameInformation */ > - 0, /* FileIdInformation */ > + sizeof(FILE_ID_INFORMATION), /* FileIdInformation */ > 0, /* FileIdExtdDirectoryInformation */ > 0, /* FileReplaceCompletionInformation */ > 0, /* FileHardLinkFullIdInformation */ > @@ -2619,6 +2619,32 @@ NTSTATUS WINAPI NtQueryInformationFile( HANDLE hFile, PIO_STATUS_BLOCK io, > } > } > break; > + case FileIdInformation: > + { > + FILE_ID_INFORMATION *info = ptr; > + memset(info, 0, sizeof(*info)); > + if (fstat(fd, &st) == -1) > + { > + io->u.Status = FILE_GetNtStatus(); > + } > + else > + { > +#if defined(linux) && defined(HAVE_FSTATFS) && defined(HAVE_STRUCT_STATFS_F_FSID___VAL) > + struct statfs volstats; > + if (fstatfs(fd, &volstats) == -1) > + { > + io->u.Status = FILE_GetNtStatus(); > + break; > + } > + > + memcpy(&info->VolumeSerialNumber, > + &volstats.f_fsid.__val, > + min(sizeof(info->VolumeSerialNumber), sizeof(volstats.f_fsid.__val))); I didn't think about this earlier, but if you are planning to use memcpy anyway, wouldn't it be easier to just copy f_fsid directly, instead of f_fsid.__val? When you do not make any assumptions about f_fsid it would work on all platforms. > +#endif > + ((LARGE_INTEGER *)&info->FileId)->QuadPart = st.st_ino; > + } > + } > + break; > default: > FIXME("Unsupported class (%d)\n", class); > io->u.Status = STATUS_NOT_IMPLEMENTED; > diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c > index 57d7df9..00856ab 100644 > --- a/dlls/ntdll/tests/file.c > +++ b/dlls/ntdll/tests/file.c > @@ -1444,6 +1444,65 @@ static void test_file_basic_information(void) > CloseHandle( h ); > } > > +static void test_file_id_information(void) > +{ > + IO_STATUS_BLOCK io; > + FILE_ID_INFORMATION fid1, fid2; > + HANDLE h = INVALID_HANDLE_VALUE; > + NTSTATUS res; > + char path[MAX_PATH], buffer[MAX_PATH]; > + > + /* Generate a temp file name */ Not all of these steps need an explanation, so you could remove some of the comments for things which are obvious. > + GetTempPathA(MAX_PATH, path); > + GetTempFileNameA(path, "foo", 0, buffer); > + > + /* Create the temp file */ > + h = CreateFileA(buffer, GENERIC_WRITE, 0, NULL, CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, 0); > + ok(h != INVALID_HANDLE_VALUE, "CreateFile error %d\n", GetLastError()); > + > + /* Query the temp file unique id */ > + memset(&fid1, 0, sizeof(fid1)); > + res = pNtQueryInformationFile(h, &io, &fid1, sizeof fid1, FileIdInformation); Here you are mixing two styles for sizeof, one with "()" and one without. The same also applies to a couple of other places below. > + if ((res == STATUS_NOT_IMPLEMENTED) || (res == STATUS_INVALID_INFO_CLASS)) The extra "()" is not really necessary. > + { > + CloseHandle(h); > + DeleteFileA(buffer); > + win_skip("FileIdInformation not supported\n"); > + return; > + } > + > + ok(res == STATUS_SUCCESS, "can't get fid1, res %x\n", res); > + > + CloseHandle(h); > + > + /* Open the file again */ > + h = CreateFileA(buffer, GENERIC_WRITE, 0, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0); > + ok(h != INVALID_HANDLE_VALUE, "CreateFile error %d\n", GetLastError()); > + > + /* Query again the temp file unique id */ > + memset(&fid2, 0, sizeof(fid2)); > + res = pNtQueryInformationFile(h, &io, &fid2, sizeof fid2, FileIdInformation); > + ok(res == STATUS_SUCCESS, "can't get fid2, res %x\n", res); > + > + /* Verify we got the same unique id for the file */ > + ok(memcmp(&fid1, &fid2, sizeof fid1) == 0, "file id is not unique\n"); > + > + CloseHandle(h); > + > + h = create_temp_file(0); > + ok(h != NULL, "error creating new file %d\n", GetLastError()); > + > + memset(&fid2, 0, sizeof(fid2)); > + res = pNtQueryInformationFile(h, &io, &fid2, sizeof fid2, FileIdInformation); > + ok(res == STATUS_SUCCESS, "can't get new fid2, res %x\n", res); > + > + /* Verify we got the different unique id for the new file */ > + ok(memcmp(&fid1, &fid2, sizeof fid1) != 0, "file id is not unique\n"); > + > + CloseHandle(h); > + DeleteFileA(buffer); > +} > + > static void test_file_all_information(void) > { > IO_STATUS_BLOCK io; > @@ -4262,4 +4321,5 @@ START_TEST(file) > test_file_disposition_information(); > test_query_volume_information_file(); > test_query_attribute_information_file(); > + test_file_id_information(); > } > diff --git a/include/winternl.h b/include/winternl.h > index 891b6a7..2f2ea86 100644 > --- a/include/winternl.h > +++ b/include/winternl.h > @@ -571,6 +571,15 @@ typedef struct _FILE_INTERNAL_INFORMATION { > LARGE_INTEGER IndexNumber; > } FILE_INTERNAL_INFORMATION, *PFILE_INTERNAL_INFORMATION; > > +typedef struct _FILE_ID_128 { > + UCHAR Identifier[16]; > +} FILE_ID_128, *PFILE_ID_128; > + > +typedef struct _FILE_ID_INFORMATION { > + ULONGLONG VolumeSerialNumber; > + FILE_ID_128 FileId; > +} FILE_ID_INFORMATION, *PFILE_ID_INFORMATION; > + > typedef struct _FILE_EA_INFORMATION { > ULONG EaSize; > } FILE_EA_INFORMATION, *PFILE_EA_INFORMATION; > -- 2.9.3 >

1 0

RE: [PATCH 8/9] server: Added server side mamed pipe read and write implementation and use it for message mode pipes.
by wylda＠volny.cz 10 Feb '17

10 Feb '17

Hi, mamed vs. named? W.

1 0

Re: ntdll: Fixed LdrUnloadDll may recursion unload same dll
by Sebastian Lackner 10 Feb '17

10 Feb '17

On 09.02.2017 10:39, 谢威 wrote: > From 3b5efb522f3ef46b009e6a159a605e087e8841a9 Mon Sep 17 00:00:00 2001 > From: Wei xie <xiewei(a)linuxdeepin.com> > Date: Thu, 19 Jan 2017 14:55:44 +0800 > Subject: [PATCH] ntdll: Fixed LdrUnloadDll may recursion unload same dll > > Signed-off-by: Wei xie <xiewei(a)linuxdeepin.com> > --- > dlls/ntdll/loader.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) > > diff --git a/dlls/ntdll/loader.c b/dlls/ntdll/loader.c > index 74feb97..8762917 100644 > --- a/dlls/ntdll/loader.c > +++ b/dlls/ntdll/loader.c > @@ -64,7 +64,7 @@ WINE_DECLARE_DEBUG_CHANNEL(pid); > typedef DWORD (CALLBACK *DLLENTRYPROC)(HMODULE,DWORD,LPVOID); > > static BOOL process_detaching = FALSE; /* set on process detach to avoid deadlocks with thread detach */ > -static int free_lib_count; /* recursion depth of LdrUnloadDll calls */ > +static int free_lib_count = 0; /* recursion depth of LdrUnloadDll calls */ This isn't really necessary, all static variables are initialized to zero. > > static const char * const reason_names[] = > { > @@ -2859,7 +2859,7 @@ static void MODULE_FlushModrefs(void) > * > * The loader_section must be locked while calling this function. > */ > -static void MODULE_DecRefCount( WINE_MODREF *wm ) > +static void MODULE_DecRefCount( WINE_MODREF *wm , BOOL bDeps) > { > int i; > > @@ -2872,13 +2872,13 @@ static void MODULE_DecRefCount( WINE_MODREF *wm ) > --wm->ldr.LoadCount; > TRACE("(%s) ldr.LoadCount: %d\n", debugstr_w(wm->ldr.BaseDllName.Buffer), wm->ldr.LoadCount ); > > - if ( wm->ldr.LoadCount == 0 ) > + if ( wm->ldr.LoadCount == 0 && bDeps) > { > wm->ldr.Flags |= LDR_UNLOAD_IN_PROGRESS; > > for ( i = 0; i < wm->nDeps; i++ ) > if ( wm->deps[i] ) > - MODULE_DecRefCount( wm->deps[i] ); > + MODULE_DecRefCount( wm->deps[i] , TRUE ); > > wm->ldr.Flags &= ~LDR_UNLOAD_IN_PROGRESS; > } > @@ -2893,6 +2893,7 @@ NTSTATUS WINAPI LdrUnloadDll( HMODULE hModule ) > { > WINE_MODREF *wm; > NTSTATUS retv = STATUS_SUCCESS; > + static HMODULE first_recursion_module = 0; > > if (process_detaching) return retv; > > @@ -2900,13 +2901,19 @@ NTSTATUS WINAPI LdrUnloadDll( HMODULE hModule ) > > RtlEnterCriticalSection( &loader_section ); > > + if (0 == free_lib_count) > + first_recursion_module = hModule; > + > free_lib_count++; > if ((wm = get_modref( hModule )) != NULL) > { > TRACE("(%s) - START\n", debugstr_w(wm->ldr.BaseDllName.Buffer)); > > /* Recursively decrement reference counts */ > - MODULE_DecRefCount( wm ); > + if (0 != free_lib_count && first_recursion_module == hModule) > + MODULE_DecRefCount( wm , FALSE ); > + else > + MODULE_DecRefCount( wm , TRUE ); At the location above free_lib_count will always be != 0 - this means that your patch basically disables unloading of dependencies completely. Do you have a small testcase for the problem you are trying to fix? > > /* Call process detach notifications */ > if ( free_lib_count <= 1 ) > @@ -2922,6 +2929,9 @@ NTSTATUS WINAPI LdrUnloadDll( HMODULE hModule ) > > free_lib_count--; > > + if (0 == free_lib_count) > + first_recursion_module = 0; > + > RtlLeaveCriticalSection( &loader_section ); > > return retv; > -- 2.10.2 > > >

1 0

Re: [PATCH 06/16] ntdll: Added support for immediate success returns in server_read_file.
by Alexandre Julliard 10 Feb '17

10 Feb '17

Jacek Caban <jacek(a)codeweavers.com> writes: > @@ -588,10 +588,22 @@ static NTSTATUS server_read_file( HANDLE handle, HANDLE event, PIO_APC_ROUTINE a > if (wait_handle) > { > NtWaitForSingleObject( wait_handle, (options & FILE_SYNCHRONOUS_IO_ALERT), NULL ); > - status = io->u.Status; > NtClose( wait_handle ); > + return io->u.Status; > + } > + > + if (status == STATUS_SUCCESS || status == STATUS_BUFFER_OVERFLOW) > + { > + io->u.Status = status; > + io->Information = total; > + if (event) NtSetEvent( event, NULL ); > + if (apc) NtQueueApcThread( GetCurrentThread(), (PNTAPCFUNC)apc, > + (ULONG_PTR)apc_context, (ULONG_PTR)io, 0 ); > } > + else if (status != STATUS_PENDING && event) > + NtResetEvent( event, NULL ); > > + if (cvalue && status != STATUS_PENDING) NTDLL_AddCompletion( handle, cvalue, status, total ); I think it would be cleaner to do these on the server-side, the same way it's done for the asynchronous case. -- Alexandre Julliard julliard(a)winehq.org

2 1

Re: [PATCH 2/2] d2d1: Added support for ID2D1GdiInteropRenderTarget interface
by Henri Verbeet 10 Feb '17

10 Feb '17

On 10 February 2017 at 13:00, Nikolay Sivov <nsivov(a)codeweavers.com> wrote: > +static HRESULT STDMETHODCALLTYPE d2d_gdi_interop_render_target_GetDC(ID2D1GdiInteropRenderTarget *iface, > + D2D1_DC_INITIALIZE_MODE mode, HDC *dc) > +{ > + struct d2d_d3d_render_target *render_target = impl_from_ID2D1GdiInteropRenderTarget(iface); > + IDXGISurface1 *surface; > + HRESULT hr; > + > + TRACE("iface %p, mode %d, dc %p.\n", iface, mode, dc); > + > + if (FAILED(hr = d2d_d3d_render_target_get_surface(render_target, &surface))) > + return hr; > + > + hr = IDXGISurface1_GetDC(surface, mode != D2D1_DC_INITIALIZE_MODE_COPY, dc); > + IDXGISurface1_Release(surface); > + > + return hr; > +} > + > +static HRESULT STDMETHODCALLTYPE d2d_gdi_interop_render_target_ReleaseDC(ID2D1GdiInteropRenderTarget *iface, > + const RECT *update) > +{ > + struct d2d_d3d_render_target *render_target = impl_from_ID2D1GdiInteropRenderTarget(iface); > + IDXGISurface1 *surface; > + HRESULT hr; > + > + TRACE("iface %p, update rect %s.\n", iface, wine_dbgstr_rect(update)); > + > + if (FAILED(hr = d2d_d3d_render_target_get_surface(render_target, &surface))) > + return hr; > + > + hr = IDXGISurface1_ReleaseDC(surface, update); This discards "const". > + IDXGISurface1_Release(surface); > + > + return hr; > +} > + Should this sync/flush in case of e.g. a WIC bitmap render target?

1 0

Re: [PATCH 1/7] gdiplus: Make GpBitmap->bitmapbits access thread safe
by Vincent Povirk 10 Feb '17

10 Feb '17

Copying the lock value in move_bitmap seems wrong to me, as it breaks any locks on the destination. Locking the source doesn't make sense because the caller is supposed to control the only reference to it. I'm not sure it makes sense in free_image_data either, as it leaves the image in an invalid state. Anything calling it should have a lock if this is important. I think it is possible to get unlock != true, when GdipImageRotateFlip calls GdipBitmapLockBits.

2 1

Re: [PATCH 2/7] gdiplus: Initialize bitmap bits for reading in GdipBitmapLockBits when ImageFlagsReadOnly is specified
by Dmitry Timoshkov 10 Feb '17

10 Feb '17

Piotr Caban <piotr(a)codeweavers.com> wrote: > /* Make sure we can convert to the requested format. */ > - if (flags & ImageLockModeRead) > + if ((flags & ImageLockModeRead) || (bitmap->image.flags & ImageFlagsReadOnly)) Is there a test for this behaviour? -- Dmitry.

2 3

Re: Participating in Outreachy
by Austin English 10 Feb '17

10 Feb '17

Thanks all! -- -Austin GPG: 14FB D7EA A041 937B On Feb 9, 2017 6:55 AM, "Aric Stewart" <aric(a)codeweavers.com> wrote: On 2/9/17 2:16 AM, Michael Stefaniuc wrote: > On 02/08/2017 11:07 PM, Jeremy White wrote: >> On 02/08/2017 03:45 PM, Marcus Meissner wrote: >>> On Wed, Feb 08, 2017 at 11:12:08AM -0700, Erich E. Hoover wrote: >>>> On Wed, Feb 8, 2017 at 11:00 AM, André Hentschel <nerv(a)dawncrow.de> wrote: >>>>> ... >>>>> The website says 6500$ or sponsored by another org: >>>>> https://wiki.gnome.org/Outreachy/Admin/InfoForOrgs#Action >>>> >>>> Clearly I should have read further ;) I don't know how our benevolent >>>> funding overlords feel about this, but it seems like a good idea to >>>> me. We did spend a lot of time at WineConf trying to figure out ways >>>> to get more people to participate, maybe this project can help. >>> >>> If the 6500 US$ figure is the minimum amount of money, then I would say no. >> >> Sorry, I was coy earlier; I was waiting on some feedback from Karen. >> >> To be clear, CodeWeavers will sponsor it and provide the $6,500. So if > Thank you very much! > >> we have someone to volunteer to coordinate, and a few people willing to >> mentor, we should be good. (I already have Stefan, so we have game >> related stuff covered). > I can mentor too and/or be a backup for Austin on the coordination part. > Though for the mentoring part the usual disclaimer applies: my Wine > technical knowledge is rather broad but very shallow. > > bye > michael > > I am willing and interested in mentoring. I have not been as active recently but still feel like I could help. Sign me up. -aric

1 0