December 2014 - Wine-Devel - list.winehq.org

Re: [PATCH 15/15] api-ms-win-core-winrt-string: Implement a few HSTRING functions.

by Martin Storsjö

Hi Sebastian, On Mon, 1 Dec 2014, Sebastian Lackner wrote: > On 01.12.2014 11:27, Martin Storsjo wrote: >> + >> + buf = malloc(sizeof(*buf)); >> + if (!buf) >> + return E_OUTOFMEMORY; >> + buf->buffer = malloc((length + 1) * sizeof(*buf->buffer)); >> + if (!buf->buffer) { >> + free(buf); >> + return E_OUTOFMEMORY; >> + } > > Some of your previous patches also contain changes to ./configure which > is an autogenerated file. Afaik its not allowed to include that when > submitting patches. Ok, I'll keep that in mind (and I can resubmit them without those changes if that's requested). > Feedback for this part: > > * HSTRING and HSTRING_HEADER should probably be defined in some include > fine. To ensure that the size of private structure matches you could > also use C_ASSERT(...), and maybe even disable align of the structure > fields. Right, they should probably go into hstring.h and winstring.h then. (One needs to take more care about the HSTRING_HEADER struct if making it public though, since the public definition should match the right size in the ABI and not have any public meaningful members, only placeholder to make it a certain size, while the internals of the DLL uses that to store certain fields.) > * You should avoid using malloc(...) in Wine source Ok, I should have guessed that. Is it ok to use memcpy for copying data though? > * http://msdn.microsoft.com/en-us/library/br224632(v=vs.85).aspx mentions: > """Calling WindowsDeleteString decrements the reference count of the backing buffer, and if the reference count reaches 0, the Windows Runtime de-allocates the buffer.""" > > I don't see the concept of refcounted backing buffers anywhere in your > code, which is most likely wrong. Its difficult to fix that later. I had a really hard time understanding the MSDN part here - I don't find any corresponding function for increasing a refcount, so I don't see how the HSTRING itself would be refcounted. So I think that only matters once you pass those string handles into the full Windows Runtime itself. I don't see how it would be hard to add that to the internal representation of HSTRING later though - as long as that representation is purely internal we should be able to change it. And if it needs to be synced with something else (e.g. Windows Runtime accessing data from within HSTRING directly without going through the functions here) we'd need to sync with some unofficial/unknown/unspecified ABI. > * I would suggest starting with some stub functions, and then add tests > before sending the actual implementation. That sounds like a sensible approach, although it'll probably take some time before I've got time to do it that way. This patch is pretty much separate from the rest of this patchset anyway (and I've got much less concrete need for it, contrary to the other patches), so I don't mind if it's deferred/dropped for now, separate from the rest at least. // Martin

Dec. 1, 2014

Re: [PATCH 15/15] api-ms-win-core-winrt-string: Implement a few HSTRING functions.

by Sebastian Lackner

On 01.12.2014 11:27, Martin Storsjo wrote: > + > + buf = malloc(sizeof(*buf)); > + if (!buf) > + return E_OUTOFMEMORY; > + buf->buffer = malloc((length + 1) * sizeof(*buf->buffer)); > + if (!buf->buffer) { > + free(buf); > + return E_OUTOFMEMORY; > + } Some of your previous patches also contain changes to ./configure which is an autogenerated file. Afaik its not allowed to include that when submitting patches. Feedback for this part: * HSTRING and HSTRING_HEADER should probably be defined in some include fine. To ensure that the size of private structure matches you could also use C_ASSERT(...), and maybe even disable align of the structure fields. * You should avoid using malloc(...) in Wine source * http://msdn.microsoft.com/en-us/library/br224632(v=vs.85).aspx mentions: """Calling WindowsDeleteString decrements the reference count of the backing buffer, and if the reference count reaches 0, the Windows Runtime de-allocates the buffer.""" I don't see the concept of refcounted backing buffers anywhere in your code, which is most likely wrong. Its difficult to fix that later. * I would suggest starting with some stub functions, and then add tests before sending the actual implementation. BTW: Michael Müller was also working on some api-ms-win-core-* stubs, but I think he is also fine when your patches get upstream instead: https://github.com/wine-compholio/wine-staging/tree/master/patches/api-ms-w… Regards, Sebastian

Dec. 1, 2014

Re: ntdll: Adds support for custom serial port baudrates on Mac

by Nikolay Sivov

On 11/29/2014 07:53 PM, Nathan Fearnley wrote: > Mac OS X supports a special function call for custom baud rates. I have > implemented this feature to allow non-standard baud rates. > Unfortunately, setting any other parameters using tcsetattr will fail, > because it does not support the higher baud rates. In order to work > around this, when setting other parameters, the baud rate is temporarily > set to 9600, then reset back to custom rate after the changes are complete. > > I can't comment on functional part, but regarding code structure this should be better separated imho. Maybe it's even better to duplicate whole functions and ifdef them all in case when __APPLE__ is not defined. Also don't use C++ 'bool', use BOOL with TRUE/FALSE defines.

Dec. 1, 2014

Re: [PATCH] wine.inf: Add quotes to webbrowser command key for mailto protocol.

by Qian Hong

Hi, `git grep command | grep HKCR | grep -v '"""'` exposed more places which seems missing quotes in command key, anyone know if we should update every place? Any advice is great appreciated! loader/wine.inf.in:HKCR,chm.file\shell\open\command,,2,"%10%\hh.exe %1" loader/wine.inf.in:HKCR,cplfile\shell\cplopen\command,,2,"rundll32.exe shell32.dll,Control_RunDLL ""%1"",%*" loader/wine.inf.in:HKCR,hlpfile\shell\open\command,,2,"%11%\winhlp32.exe %1" loader/wine.inf.in:HKCR,inffile\shell\install\command,,2,"%11%\rundll32.exe setupapi,InstallHinfSection DefaultInstall 132 %1" loader/wine.inf.in:HKCR,inifile\shell\open\command,,2,"%11%\notepad.exe %1" loader/wine.inf.in:HKCR,inifile\shell\print\command,,2,"%11%\notepad.exe /p %1" loader/wine.inf.in:HKCR,rtffile\shell\open\command,,2,"%11%\wordpad.exe %1" loader/wine.inf.in:HKCR,rtffile\shell\print\command,,2,"%11%\wordpad.exe /p %1" loader/wine.inf.in:HKCR,txtfile\shell\open\command,,2,"%11%\notepad.exe %1" loader/wine.inf.in:HKCR,txtfile\shell\print\command,,2,"%11%\notepad.exe /p %1" loader/wine.inf.in:HKCR,wrifile\shell\open\command,,2,"%11%\wordpad.exe %1" loader/wine.inf.in:HKCR,wrifile\shell\print\command,,2,"%11%\wordpad.exe /p %1" On Mon, Dec 1, 2014 at 1:38 PM, Qian Hong <qhong(a)codeweavers.com> wrote: > Hi Piotr, > > This is a follow up of the below commit: > > commit 54833c9b7cf5f8f5e37b43038e1fa4c1c369deb0 > Author: Piotr Caban <piotr(a)codeweavers.com> > Date: Wed May 4 13:20:41 2011 +0200 > > wine.inf: Add quotes to webbrowser command keys. > > To make sure i'm not breaking anything which is intended, would you mind > have a look? > > Thanks! > > --- > loader/wine.inf.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > -- Regards, Qian Hong - http://www.codeweavers.com

Dec. 1, 2014