[webkit-reviews] review denied: [Bug 47498] Crash while processing ill-formed SVG with cycles. : [Attachment 70819] First attempt to fix

[bugzilla-daemon at webkit.org]

Csaba Osztrogonac <ossy at webkit.org> has denied Cosmin Truta
<ctruta at chromium.org>'s request for review:
Bug 47498: Crash while processing ill-formed SVG with cycles.
https://bugs.webkit.org/show_bug.cgi?id=47498

Attachment 70819: First attempt to fix
https://bugs.webkit.org/attachment.cgi?id=70819&action=review

------- Additional Comments from Csaba Osztrogonac <ossy at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=70819&action=review

I'm not familiar with SVG, so I can't review the logic of the patch.
r- due to build error

> WebCore/rendering/SVGResources.cpp:453
> +    RenderSVGResourceType resourceType = clipper->resourceType();
> +    ASSERT(resourceType == ClipperResourceType);
> +

To avoid "unused variable" warning(error), you should 
use ASSERT_UNUSED, or don't use a new local variable.

But I propose avoiding unnecessary local variable,
which is dead code in relase mode:

ASSERT(clipper->resourceType() == ClipperResourceType);

> WebCore/rendering/SVGResources.cpp:476
> +    RenderSVGResourceType resourceType = filter->resourceType();
> +    ASSERT(resourceType == FilterResourceType);
> +

ditto

> WebCore/rendering/SVGResources.cpp:498
> +    RenderSVGResourceType resourceType = markerStart->resourceType();
> +    ASSERT(resourceType == MarkerResourceType);

ditto

> WebCore/rendering/SVGResources.cpp:520
> +    RenderSVGResourceType resourceType = markerMid->resourceType();
> +    ASSERT(resourceType == MarkerResourceType);

ditto

> WebCore/rendering/SVGResources.cpp:542
> +    RenderSVGResourceType resourceType = markerEnd->resourceType();
> +    ASSERT(resourceType == MarkerResourceType);

ditto

> WebCore/rendering/SVGResources.cpp:564
> +    RenderSVGResourceType resourceType = masker->resourceType();
> +    ASSERT(resourceType == MaskerResourceType);

ditto

> WebCore/rendering/SVGResources.cpp:586
> +    RenderSVGResourceType resourceType = fill->resourceType();
> +    ASSERT(resourceType == PatternResourceType || resourceType ==
LinearGradientResourceType || resourceType == RadialGradientResourceType);

ditto

> WebCore/rendering/SVGResources.cpp:608
> +    RenderSVGResourceType resourceType = stroke->resourceType();
> +    ASSERT(resourceType == PatternResourceType || resourceType ==
LinearGradientResourceType || resourceType == RadialGradientResourceType);

ditto