On Thu, 18 Dec 2003, Sven Neumann wrote:
> Hi,
>
> Shlomi Fish <shlomif@xxxxxxxxxxxxxxxxxxx> writes:
>
> > Well, here is the up-to-date patch according to the commentary from
> > the core developers that I received on the IRC.
>
> Please see my comments on the patch and put a revised version to
> Bugzilla.
>
> > +{
> > + GimpGradientSegment *seg;
> > + int i;
> > +
> > + if (i < 0)
> > + {
> > + return NULL;
> > + }
>
> I think you want to check index here, not the uninitialized variable
> i. Better make this a g_return_val_if_fail() though. In general you
> should add such checks for all assumptions you make about function
> parameters.
>
You are right - it was a bug. Fixed.
>
> > +void gimp_gradient_get_segment_left_color (GimpGradient *gradient,
> > + GimpGradientSegment *seg,
> > + GimpRGB * color)
>
> Please try to follow the coding style and align function parameters
> the same way all other GIMP functions do it. The same comment applies
> to the declarations in the header file.
>
Done.
> > + gimp_data_dirty (GIMP_DATA(gradient));
>
> Please insert a space before the opening bracket.
>
OK.
This fixes will be incorporated in the next version of the patch, which
will also have position set/get routines.
Regards,
Shlomi Fish
----------------------------------------------------------------------
Shlomi Fish shlomif@xxxxxxxxxxxxxxxxxxx
Home Page: http://t2.technion.ac.il/~shlomif/
An apple a day will keep a doctor away. Two apples a day will keep two
doctors away.
Falk Fish
