+static GpStatus init_container(GraphicsContainerItem** container,
GDIPCONST GpGraphics* graphics){
- GpStatus sts;
- if(!container || !graphics)
return InvalidParameter;
Since this is an internal function and you know container and graphics will always be non-NULL when you call it, you don't need to make this check.
- sts = GdipDeleteMatrix(graphics->worldtrans);
- if(sts != Ok)
return sts;
- sts = GdipCloneMatrix(container->worldtrans, &graphics->worldtrans);
- if(sts != Ok)
return sts;
If the delete succeeds but the clone fails, you'll have a Graphics with a deleted world transform. You should allocate the objects you need before freeing those attached to the Graphics.
I think that in this case you could get away with copying pointers rather than creating new objects, and you can assume the delete functions succeed. Since the container is about to be destroyed, you would need to make sure the attached matrix and region are not destroyed with it.
- /* did not find a matching container */
- if(&container->entry == &graphics->containers)
return Ok;
That's a little strange, even though the test shows it's not an error. Personally, I would throw in a WARN for this case.
I assume you meant to mark this as [1/2] and the test as [2/2].
This looks good to me. It's almost at the point where I can't predict whether it will be accepted (as opposed to being sure it won't), but Alexandre nailed me once for doing a similar "delete before allocating the replacement" thing. So I assume he cares about that.