Re: [PATCH 00/11] first batch of psppsheet changes

[Ben Pfaff]

John Darrington <address@hidden> writes:

> These all look fine to me, except the last one:
>
> diff --git a/src/ui/gui/psppire-dict.c b/src/ui/gui/psppire-dict.c
> index 04bd3e3..5c6cfeb 100644
> --- a/src/ui/gui/psppire-dict.c
> +++ b/src/ui/gui/psppire-dict.c
> @@ -473,7 +473,7 @@ psppire_dict_get_variable (const PsppireDict *d, gint idx)
>    g_return_val_if_fail (d, NULL);
>    g_return_val_if_fail (d->dict, NULL);
>
> -  if ( dict_get_var_cnt (d->dict) <= idx )
> +  if ( idx < 0 || dict_get_var_cnt (d->dict) <= idx )
>      return NULL;
>
> I'm kinda interested to know why we're silently returning NULL anyway,
> and not using g_return_val_if_fail.   Most probably this is/was a kludge
> to avoid some other problem.  Perhaps it is no longer necessary.  Anyway,
> I'd be interested to see what happens if we change it to use 
> g_return_val_if_fail.

Thanks for the reviews.  I pushed all of the commits but that
one.  I'll take a look at its callers to see whether we can just
switch to g_return_val_if_fail and put the new patch at the
beginning of my next series.