Re: windowscodecs: Fix *_CopyPixels functions to properly handle a NULL rectangle
+1 + if (!prc) + { + UINT width, height; + hr = IWICBitmapSource_GetSize(This->source, &width, &height); + if (FAILED(hr)) return hr; + rc.X = 0; + rc.Y = 0; + rc.Width = width; + rc.Height = height; + prc = &rc; + } Any reason you didn't use this approach in the other functions?
W dniu 17.10.2010 01:45, Vincent Povirk pisze:
+1
+ if (!prc) + { + UINT width, height; + hr = IWICBitmapSource_GetSize(This->source,&width,&height); + if (FAILED(hr)) return hr; + rc.X = 0; + rc.Y = 0; + rc.Width = width; + rc.Height = height; + prc =&rc; + }
Any reason you didn't use this approach in the other functions?
I don't know really. Doing a memcpy to a local rectangle seems a more natural way to do it, as I don't really like to modify input arguments. In this case I guess I just blindly copied an existing solution that I found later during the review of the other codecs. If you want I can unify the implementations so that all of them look like the one above.
On 10/17/2010 01:59 AM, Krzysztof Nowicki wrote:
Doing a memcpy to a local rectangle seems a morenatural way to do it
Not really. Doing RECT = RECT is the natural way to do it. Don't use memcpy to copy one structure to another structure of the same type. Vitaliy.
W dniu 17.10.2010 18:49, Vitaliy Margolen pisze:
On 10/17/2010 01:59 AM, Krzysztof Nowicki wrote:
Doing a memcpy to a local rectangle seems a morenatural way to do it
Not really. Doing RECT = RECT is the natural way to do it. Don't use memcpy to copy one structure to another structure of the same type.
Vitaliy.
Where I come from copying structures directly is considered bad practice and it's safer to use memcpy. We had problems before with broken compilers that would try to do some black magic in such cases. So the policy was to use memcpy for structures because it's safer. A good compiler will inline such a memcpy anyway. If the policy for Wine is to copy structures directly then I'll respect it. K.
On Mon, Oct 18, 2010 at 08:51:49PM +0200, Krzysztof Nowicki wrote:
W dniu 17.10.2010 18:49, Vitaliy Margolen pisze:
On 10/17/2010 01:59 AM, Krzysztof Nowicki wrote:
Doing a memcpy to a local rectangle seems a morenatural way to do it
Not really. Doing RECT = RECT is the natural way to do it. Don't use memcpy to copy one structure to another structure of the same type.
Vitaliy.
Where I come from copying structures directly is considered bad practice and it's safer to use memcpy. We had problems before with broken compilers that would try to do some black magic in such cases. So the policy was to use memcpy for structures because it's safer. A good compiler will inline such a memcpy anyway.
Actually, for a normal application, the compiler knows what memcpy() does and can make assumptions based on the actual structure type involved. This can cause extreme grief if you try to copy from a misaligned pointer into a local variable (strictly, in valid C, you can never get a misaligned pointer - and the compiler will use word accesses). David -- David Laight: david(a)l8s.co.uk
On 10/18/2010 12:51 PM, Krzysztof Nowicki wrote:
Where I come from copying structures directly is considered bad practice and it's safer to use memcpy. We had problems before with broken compilers that would try to do some black magic in such cases. Then don't use broken compilers. GCC handles struct copying this just fine. And it's up-to compiler what sort of magic it's uses white or black. As long as struct gets copied you are all set.
If the policy for Wine is to copy structures directly then I'll respect it. I'm not the one setting policies, but that's what you should do - utilize C language constructs instead of going around them.
Vitaliy.
On 10/18/10 9:53 PM, Vitaliy Margolen wrote:
On 10/18/2010 12:51 PM, Krzysztof Nowicki wrote:
Where I come from copying structures directly is considered bad practice and it's safer to use memcpy. We had problems before with broken compilers that would try to do some black magic in such cases. Then don't use broken compilers. GCC handles struct copying this just fine. And it's up-to compiler what sort of magic it's uses white or black. As long as struct gets copied you are all set.
Actually, gcc modifies the code to do a PROPER memcpy. Thus you can copy the contents of a string using a pointer rather than the string if you use variable a = variable b. Using memcpy might lead to interesting results if you do the same thing.
If the policy for Wine is to copy structures directly then I'll respect it. I'm not the one setting policies, but that's what you should do - utilize C language constructs instead of going around them.
That's very true and this has been a C language construct for quite some time. James McKenzie
participants (5)
-
David Laight -
James McKenzie -
Krzysztof Nowicki -
Vincent Povirk -
Vitaliy Margolen