Re: [PATCH] quartz: Use straight assignments instead of CopyRect().

[Michael Stefaniuc]

On 11/22/2016 08:01 PM, Hadrien Boizard wrote:
> Signed-off-by: Hadrien Boizard
>
> dlls/quartz/videorenderer.c | 8 ++++----
>  dlls/quartz/vmr9.c          | 8 ++++----
>  2 files changed, 8 insertions(+), 8 deletions(-)
>
> 0001-quartz-Use-straight-assignments-instead-of-CopyRect.patch
>
> diff --git a/dlls/quartz/videorenderer.c b/dlls/quartz/videorenderer.c
> index 61f08f5..6631ccd 100644
> --- a/dlls/quartz/videorenderer.c
> +++ b/dlls/quartz/videorenderer.c
> @@ -483,7 +483,7 @@ static const BaseWindowFuncTable renderer_BaseWindowFuncTable = {
>  static HRESULT WINAPI VideoRenderer_GetSourceRect(BaseControlVideo* iface, RECT *pSourceRect)
>  {
>      VideoRendererImpl *This = impl_from_BaseControlVideo(iface);
> - CopyRect(pSourceRect,&This->SourceRect);
> - *pSourceRect = This->SourceRect;

NACK, this is not a no-op change.
CopyRect() also checks for NULL rect pointers so it protects against
NULL pSourceRect. With your change apps would crash if they do that.

The proper fix is to add tests for NULL rects and return the proper
error in those functions.

Or at least add code like:
if (!pSourceRect)
   return E_POINTER;

No clue if Andrew Eikum is willing to accept that without tests.
E_INVALIDARG would be also a plausible error code.

bye
    michael