I tried build a real Windows application (mimikatz) and had troubles with some wine headers incompatibility. The patch set improves headers to allow build the foreign application with Wine.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3506
On Thu Aug 3 10:09:59 2023 +0000, Maxim Karasev wrote:
> Printing a non-english username.
> `wprintf()` by default doesn't print non-english letters at all (I
> remember testing various system LC_ALL variants, like `C.UTF-8`,
> `en_US.UTF-8`, `ru_RU.UTF-8`, and none of that took an effect).
> With `setlocale(LC_ALL, something)` it does print the letters, but
> 1. Only of a language that is set, so I presume it chooses something
> like a `cp1251` equivalent (an 8-bit language-specific encoding?).
> 2. It isn't used in any of other wine programs, while that
> `ConsoleWriteW()` hack is used at least in present whoami and xcopy
> implementations (probably more).
> This implementation, in turn, behaves exactly like the Windows whoami.
> Also when testing the `klist` I noticed that on Windows it was printing
> `?` signs instead of `Server` string (somehow hooked up a translation?),
> so `ConsoleWriteW()` seems to be necessary for any translatable console
> programs if one wants to avoid `setlocale()`.
I tried a username with non-ascii characters on Windows 10 and Wine and whoami behaves the same. Non-ascii characters are rendered on the console. When redirecting output non-ascii characters are replaced with '?'.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3473#note_41261
On Thu Aug 3 16:39:16 2023 +0000, Yuxuan Shui wrote:
> Yes, what you said makes sense.
> > something that's pretty much always been within Wine's scope to fix.
> I want to assure you I 100% understand that. After all, this MR is
> exactly trying to fix such a case.
And I think it's better if I clarify what I mean when I say "invalid".
In this example, this use of `ReleaseSRWLockExclusive` is clearly invalid w.r.t. the official description of the API, and that's why I called it so.
Real world applications do make use of APIs in invalid ways, and Wine has to support those usages. However this doesn't make such uses less invalid.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3504#note_41257
- Fixed the queue memory leak
- Created a critical section for the queue
- I removed the allocator lock when it calls NotifyRelease, I didn't add a case for being able to call NotifyRelease inside the lock because NotifyRelease never receives a pointer to the allocator so I don't think there is a need to still lock while running NotifyRelease.
- Moved process_input to NotifyRelease.
- Also removed the test because it doesn't reproduce the bug reliably specially in the testbot, but I can add it again if it's needed or try to look for a way to make it reliable.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3319#note_41256
I'm not sure how could I test this behavior, right now the test I wrote works but there are times where it doesn't enter into the deadlock.
--
v4: evr: Remove process input handling from streaming thread.
evr: Don't lock allocator in NotifyRelease call.
evr: Create critical section for sample queue.
evr: Release sample queue when streaming ends.
https://gitlab.winehq.org/wine/wine/-/merge_requests/3319
On Thu Aug 3 16:09:52 2023 +0000, Zebediah Figura wrote:
> > I feel that is a bit overkill for a single application, which is using
> the lock in an invalid way anyway.
> Maybe, but I don't like the way this patch basically feels like it's
> hacking around the current behaviour.
> I'd also assert that "invalid" is often not a very meaningful word when
> it comes to Wine. There's documented API, sure, but programs making
> assumptions about internals is commonplace, and something that's pretty
> much always been within Wine's scope to fix.
Yes, what you said makes sense.
> something that's pretty much always been within Wine's scope to fix.
I want to assure you I 100% understand that. After all, this MR is exactly trying to fix such a case.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3504#note_41253
> I feel that is a bit overkill for a single application, which is using the lock in an invalid way anyway.
Maybe, but I don't like the way this patch basically feels like it's hacking around the current behaviour.
I'd also assert that "invalid" is often not a very meaningful word when it comes to Wine. There's documented API, sure, but programs making assumptions about internals is commonplace, and something that's pretty much always been within Wine's scope to fix.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3504#note_41252
On Thu Aug 3 15:49:48 2023 +0000, Yuxuan Shui wrote:
> I feel that is a bit overkill for a single application, which is using
> the lock in an invalid way anyway.
Had some discussions with @huw and agreed we do need to do this properly.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3504#note_41250
On Thu Aug 3 15:44:45 2023 +0000, Jinoh Kang wrote:
> > ending up re-using the same mapping because it is created with the
> desktop name.
> Then I think we should unlink the mapping object (but keep it alive),
> and create a *new* mapping object with the same.
> Perhaps this was obvious at first, but there was some complication that
> I failed to notice. Do you mind elaborating if this is the case?
Sure, we should perhaps unlink the mapping object too.
Although that's probably a good thing to do, I'm not sure to see how it makes any difference wrt this `shared_ptr` member, and why we should get rid of this member.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3103#note_41249