gdiplus: Simplify logic/control flow in SOFTWARE_GdipFillRegion.

Vincent Povirk madewokherd at gmail.com
Sun Apr 3 11:47:33 CDT 2011


I don't think this is an improvement. I think it's easier to use a
single variable to indicate failure in the code, and to only use that
variable when deciding whether to proceed further. This case only
works because the GdipAlloc happens to be the only operation that can
fail in that block before GdipGetRegionScansI. I worry that someone
(probably me) could modify the code so that is no longer the case and
fail to notice that the logic needs to be changed ( to check for stat
== Ok && scans, rather than just scans).

On Sat, Apr 2, 2011 at 3:36 PM, Gerald Pfeifer <gerald at pfeifer.com> wrote:
> This doesn't make things tremendously simpler, but does avoid one
> unnecessary if / condition.
>
> If you prefer, I can switch the two arms of the if to retain the
> original !scans.
>
> Gerald
>
> ---
>  dlls/gdiplus/graphics.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/gdiplus/graphics.c b/dlls/gdiplus/graphics.c
> index 4fdfa9c..49f3495 100644
> --- a/dlls/gdiplus/graphics.c
> +++ b/dlls/gdiplus/graphics.c
> @@ -3709,16 +3709,15 @@ static GpStatus SOFTWARE_GdipFillRegion(GpGraphics *graphics, GpBrush *brush,
>             if (stat == Ok && scans_count != 0)
>             {
>                 scans = GdipAlloc(sizeof(*scans) * scans_count);
> -                if (!scans)
> -                    stat = OutOfMemory;
> -
> -                if (stat == Ok)
> +                if (scans)
>                 {
>                     stat = GdipGetRegionScansI(temp_region, scans, &dummy, identity);
>
>                     if (stat != Ok)
>                         GdipFree(scans);
>                 }
> +                else
> +                    stat = OutOfMemory;
>             }
>
>             GdipDeleteMatrix(identity);
> --
> 1.7.4.1
>
>
>



More information about the wine-devel mailing list