Re: [GDI+: 1/10] first pen implementation

[Detlef Riekenberg]

On Mo, 2007-06-11 at 11:51 -0700, Evan Stade wrote:
> +GpStatus WINGDIPAPI GdipCreatePen1(ARGB color, FLOAT width, GpUnit
> unit,
> - GpPen **pen)
>
> +{
> - LOGBRUSH lb;
> - GpPen *gp_pen;
> -
> - gp_pen = (GpPen*) GdipAlloc(sizeof(GpPen));
> - if(!pen)    return OutOfMemory;
> -
> - gp_pen->style = GP_DEFAULT_PENSTYLE;

This looks not correct.
You returned "OutOfMemory", but you check "pen".
I think, you really mean "gp_pen" in that "if".

A test with pen==NULL would be nice.
> - if((gp_pen->unit == UnitWorld) || (gp_pen->unit == UnitPixel)) {
> -
> - } else {
> -
>    FIXME("UnitWorld, UnitPixel only supported units");
> -
>    return NotImplemented;

You leak the allocated memory from gp_pen here.
The FIXME does not help very much.
You should dump the value for the unsupported unit.
> - if(!gp_pen)
> -
>    return GenericError;

This is dead code in this location.
When the allocation for gp_pen failed, you will never reach this line,
because gp_pen was accesed before and produced a crash.