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

Re: [PATCH] xaudio2: Avoid redefining several F3DAudio related constants.

by Gerald Pfeifer

On Thu, 30 Apr 2020, Andrew Eikum wrote: >> In file included from ../xaudio2_7/x3daudio.c:29: >> .../include/F3DAudio.h:69: warning: "SPEAKER_2POINT1" redefined > ../../include/x3daudio.h:127: note: this is the location of the previous definition >> >> Our own include/x3daudio.h tries to prevent that via >> >> #ifndef _SPEAKER_POSITIONS_ >> #define _SPEAKER_POSITIONS_ >> >> However, current it is included *before* the system provided F3DAudio.h, >> so this safety net falls short. >> >> Fixed thusly by changing the order of these two include files so that the >> system provided one comes first. > I'm not opposed to this, but I think there's a better solution. The > platform SDK and Wine's header both check for "#ifndef SPEAKER_MONO" > before defining the speaker arrangements symbols. However, FAudio's > header checks for "#ifndef _SPEAKER_COMBINATIONS_" instead. This > symbol doesn't occur in the platform SDK. > > Perhaps we should change FAudio's header to check for SPEAKER_MONO > instead, to match the platform SDK? Yes, that looks like a valid idea. Even if you can get such a change accepted, how long until the next release and how long until that spreads widely? And it looks like an implementation detail (subject to change)? So I suggest to go with my patch for Wine now, and if you can push for such a change in FAudio upstream on top, all the better! Hope this makes sense? Gerald

April 30, 2020

[PATCH 2/2] ntoskrnl: Quiet down failed class installer debug message

by Andrew Eikum

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> --- dlls/ntoskrnl.exe/pnp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ntoskrnl.exe/pnp.c b/dlls/ntoskrnl.exe/pnp.c index 75310f7b35..9e4c2c1416 100644 --- a/dlls/ntoskrnl.exe/pnp.c +++ b/dlls/ntoskrnl.exe/pnp.c @@ -298,7 +298,7 @@ static BOOL install_device_driver( DEVICE_OBJECT *device, HDEVINFO set, SP_DEVIN { if (!SetupDiCallClassInstaller(dif_list[i], set, sp_device) && GetLastError() != ERROR_DI_DO_DEFAULT) { - ERR("Install function %#x failed, error %#x.\n", dif_list[i], GetLastError()); + WARN("Install function %#x failed, error %#x.\n", dif_list[i], GetLastError()); return FALSE; } } -- 2.26.2

April 30, 2020

[PATCH 1/2] setupapi: Quiet down missing driver debug message

by Andrew Eikum

Signed-off-by: Andrew Eikum <aeikum(a)codeweavers.com> --- dlls/setupapi/devinst.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c index 5bd2b88a02..fca60402a5 100644 --- a/dlls/setupapi/devinst.c +++ b/dlls/setupapi/devinst.c @@ -4739,7 +4739,7 @@ BOOL WINAPI SetupDiSelectBestCompatDrv(HDEVINFO devinfo, SP_DEVINFO_DATA *device if (!device->driver_count) { - ERR("No compatible drivers were enumerated for device %s.\n", debugstr_w(device->instanceId)); + WARN("No compatible drivers were enumerated for device %s.\n", debugstr_w(device->instanceId)); SetLastError(ERROR_NO_COMPAT_DRIVERS); return FALSE; } -- 2.26.2

April 30, 2020

[PATCH] winspool.drv: return success in AddPrintProcessorA

by Austin English

This syncs the return value with AddPrintProcessorW Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=26171 Signed-off-by: Austin English <austinenglish(a)gmail.com> --- dlls/winspool.drv/info.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/winspool.drv/info.c b/dlls/winspool.drv/info.c index 593f091c2d..0ddca147e6 100644 --- a/dlls/winspool.drv/info.c +++ b/dlls/winspool.drv/info.c @@ -5405,7 +5405,7 @@ BOOL WINAPI AddPrintProcessorA(LPSTR pName, LPSTR pEnvironment, LPSTR pPathName, { FIXME("(%s,%s,%s,%s): stub\n", debugstr_a(pName), debugstr_a(pEnvironment), debugstr_a(pPathName), debugstr_a(pPrintProcessorName)); - return FALSE; + return TRUE; } /***************************************************************************** -- 2.24.1

April 30, 2020

Re: [PATCH 1/2] fsutil: make usage and unsupported command messages localizable

by Zebediah Figura

This patch subject is misleading, as it implies that these messages were already present. A better subject could be "fsutil: Print a usage message when called without arguments." I also note that the usage message seems to be identical to Windows, minus unimplemented commands. In general we want to avoid copying Windows, only doing so when an application depends on it. With respect to the implementation, I think some basic tests would not be a bad idea. Some further comments: > + if (argc > 1) > + { > + FIXME("unsupported command %s\n", debugstr_w(argv[1])); > + output_string(STRING_UNSUPPORTED_CMD, argv[1]); > + ret = 1; > + } I'm not sure it makes a lot of sense to output a localized message here; I think the FIXME is enough. Same for STRING_UNSUPPORTED_PARAM. > + LoadStringW(GetModuleHandleW(NULL), msg, fmt, sizeof(fmt)/sizeof(fmt[0])); ARRAY_SIZE(fmt) Also, I know that this is copied from elsewhere, but I imagine we could simplify this code by using FORMAT_MESSAGE_FROM_HMODULE. > + WINE_TRACE(" %s", wine_dbgstr_w(argv[i])); > + WINE_TRACE("\n"); I think, though I'm not sure, that TRACE and debugstr_w() are fine in programs nowadays. > +static const WCHAR createW[]=L"create"; > +static const WCHAR hardlinkW[]=L"hardlink"; I don't think there's any reason not to just put these inline where they're used. > + if (!_wcsicmp(argv[2], createW)) This can be wciscmp, without the underscore.

April 30, 2020

Re: [PATCH 1/5] kernelbase/locale: Implement sortkey generation on official tables

by Fabian Maurer

Hello Zebediah, > Could be, though I'd warn that attempts to reduce duplication (e.g. by > introducing complicated macros) can often make code much less readable. Sure, but in this case it should be very well worth it. > Okay, I'm not sure I'm reading this macro right, but this is all > internal to diacritic weights, right? If I understand, final weights > which are 1 or 0 get stripped from the end of the sortkey, right? I > think you could easily handle that by stripping weights after adding all > of the diacritics, e.g. to use my previous example code, > > if (!(flags & NORM_IGNORENONSPACE)) > { > for (i = 0; i < srclen; ++i) > used += get_diacritic_weights(src[i], dst + used, dstlen - used); > > while (dst[used - 1] < 2) > used--; > } Generally, yes. But it not only affects diacritics. And one needs to keep in mind that the separator is also a 0 and must not be removed. But maybe here the duplication is worth it? Regards, Fabian Maurer

April 30, 2020

Re: [PATCH 1/5] kernelbase/locale: Implement sortkey generation on official tables

by Zebediah Figura

On 4/30/20 2:15 PM, Fabian Maurer wrote: > Hello Zebediah, > > >> Could be, though I'd warn that attempts to reduce duplication (e.g. by >> introducing complicated macros) can often make code much less readable. > > Sure, but in this case it should be very well worth it. > > >> Okay, I'm not sure I'm reading this macro right, but this is all >> internal to diacritic weights, right? If I understand, final weights >> which are 1 or 0 get stripped from the end of the sortkey, right? I >> think you could easily handle that by stripping weights after adding all >> of the diacritics, e.g. to use my previous example code, >> >> if (!(flags & NORM_IGNORENONSPACE)) >> { >> for (i = 0; i < srclen; ++i) >> used += get_diacritic_weights(src[i], dst + used, dstlen - used); >> >> while (dst[used - 1] < 2) >> used--; >> } > > Generally, yes. But it not only affects diacritics. And one needs to keep in > mind that the separator is also a 0 and must not be removed. > But maybe here the duplication is worth it? Sure, you'd need to guard a lower bound for "used" there in any case. > > > Regards, > Fabian Maurer > > > > >

April 30, 2020

Re: [PATCH 1/5] kernelbase/locale: Implement sortkey generation on official tables

by Zebediah Figura

On 4/30/20 1:42 PM, Fabian Maurer wrote: > Hello Zebediah, > >> Yes, you'd have to iterate over the string multiple times. However, >> that's a fixed, small number. I wouldn't be concerned about performance >> until we find evidence that performance is a concern. > > If you say so. I'll see what I can come up with. > > >> I don't think you need to write it such that you switch over which >> weight to write, either. It's probably easier just to make them separate >> functions, like in my example below. > > That would lead to quite a bunch of code duplication, especially when we come > to the more difficult to handle characters. I'd like to avoid that. A switch > for which weight to write should be a lot simpler. Could be, though I'd warn that attempts to reduce duplication (e.g. by introducing complicated macros) can often make code much less readable. > > >>> Also keep in mind, that we need to have some temporary buffer anyways, >>> since weights that are very small only get added when there is a bigger >>> one following it. Either that, or we need to backtrack and remove weights >>> again. Due to those issues, I opted for the current approach. >> >> Can you please give an example of this? I'm not sure I see it in any of >> your patches. > > It's in patch 1, inside APPEND_LIST_TO_SORTKEY, see 'statement_is_ignored'. > And AFAIK not only diacritics are affected. > Not sure how I would best handle that when writing directly to the sortkey. Okay, I'm not sure I'm reading this macro right, but this is all internal to diacritic weights, right? If I understand, final weights which are 1 or 0 get stripped from the end of the sortkey, right? I think you could easily handle that by stripping weights after adding all of the diacritics, e.g. to use my previous example code, if (!(flags & NORM_IGNORENONSPACE)) { for (i = 0; i < srclen; ++i) used += get_diacritic_weights(src[i], dst + used, dstlen - used); while (dst[used - 1] < 2) used--; } >> Generally, you can continue to calculate size but only write when >> there's space in the buffer. When I've written functions like this I've >> found it to be relatively simple, though maybe there's some details here >> that I'm missing... > > Well, there multiple places where we'd need to check. But maybe it'll work > out, just seemed more complex to me. > > >>>> Is there a reason to use LCMapStringEx() here rather than LCMapString()? >>> >>> LCMapStringA/W completely ignore the locale, and use a LCID. We'd need to >>> fix those functions to properly convert the locale. To start simple, I >>> decided to use the more low-level function and once that works, built the >>> rest on top. >> It seems a simple enough fix; just call LCIDToLocaleName(). > > Yeah, probably. But I'd rather get the low-level stuff working first. Then I > can write tests for the higher level functions and then fix them. > > >> Generally I believe that explicitly specifying "struct" is preferred as >> it makes clear what kind of object you're dealing with. > > Okay then. > > Regards, > Fabian Maurer > > >

April 30, 2020

Re: [PATCH 1/5] kernelbase/locale: Implement sortkey generation on official tables

by Fabian Maurer

Hello Zebediah, > Yes, you'd have to iterate over the string multiple times. However, > that's a fixed, small number. I wouldn't be concerned about performance > until we find evidence that performance is a concern. If you say so. I'll see what I can come up with. > I don't think you need to write it such that you switch over which > weight to write, either. It's probably easier just to make them separate > functions, like in my example below. That would lead to quite a bunch of code duplication, especially when we come to the more difficult to handle characters. I'd like to avoid that. A switch for which weight to write should be a lot simpler. > > Also keep in mind, that we need to have some temporary buffer anyways, > > since weights that are very small only get added when there is a bigger > > one following it. Either that, or we need to backtrack and remove weights > > again. Due to those issues, I opted for the current approach. > > Can you please give an example of this? I'm not sure I see it in any of > your patches. It's in patch 1, inside APPEND_LIST_TO_SORTKEY, see 'statement_is_ignored'. And AFAIK not only diacritics are affected. Not sure how I would best handle that when writing directly to the sortkey. > Generally, you can continue to calculate size but only write when > there's space in the buffer. When I've written functions like this I've > found it to be relatively simple, though maybe there's some details here > that I'm missing... Well, there multiple places where we'd need to check. But maybe it'll work out, just seemed more complex to me. > >> Is there a reason to use LCMapStringEx() here rather than LCMapString()? > > > > LCMapStringA/W completely ignore the locale, and use a LCID. We'd need to > > fix those functions to properly convert the locale. To start simple, I > > decided to use the more low-level function and once that works, built the > > rest on top. > It seems a simple enough fix; just call LCIDToLocaleName(). Yeah, probably. But I'd rather get the low-level stuff working first. Then I can write tests for the higher level functions and then fix them. > Generally I believe that explicitly specifying "struct" is preferred as > it makes clear what kind of object you're dealing with. Okay then. Regards, Fabian Maurer

April 30, 2020

Re: [PATCH v4 2/2] server: Add USD support with timestamp updates.

by Paul Gofman

On 4/30/20 21:21, Zebediah Figura wrote: > >> It may then be possible to implement time using USD but that would >> reduce the clocks accuracy down to the server update frequency. > If Windows has updates that slow, I think this makes sense. We'd > probably want to use a different channel for +timestamp, though > [QueryPerformanceCounter(), I guess?] > > There's probably other consequences I'm not considering... > It might be potentially usable for GetTickCount() which has resolution around 10-15ms (though dangerous probably), but QueryPerformanceCounter() is designed as higher resolution timer, every game is using it and expects better resolution. 15-16ms is a long time. But proper Unix functions for getting high resolution time should already be quick, they also end up getting time from shared memory pages maintained by kernel.

April 30, 2020