On Thu, 18 Dec 2003, Sven Neumann wrote:

> Hi,
>
> Shlomi Fish <[EMAIL PROTECTED]> 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        [EMAIL PROTECTED]
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
_______________________________________________
Gimp-developer mailing list
[EMAIL PROTECTED]
http://lists.xcf.berkeley.edu/mailman/listinfo/gimp-developer

Reply via email to