[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/11] first batch of psppsheet changes
From: |
Ben Pfaff |
Subject: |
Re: [PATCH 00/11] first batch of psppsheet changes |
Date: |
Mon, 16 Apr 2012 20:21:00 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
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.
- [PATCH 03/11] psppire-data-store: Use NULL for a null pointer., (continued)
- [PATCH 03/11] psppire-data-store: Use NULL for a null pointer., Ben Pfaff, 2012/04/15
- [PATCH 02/11] psppire-data-store: Use PSPPIRE namespace instead of GTK+'s., Ben Pfaff, 2012/04/15
- [PATCH 06/11] find-dialog: Change "Cancel" button to "Close" button., Ben Pfaff, 2012/04/15
- [PATCH 05/11] gtkxpaned: Remove write-only variables., Ben Pfaff, 2012/04/15
- [PATCH 08/11] psppire-dict: Get rid of static var in auto_generate_var_name()., Ben Pfaff, 2012/04/15
- [PATCH 07/11] Allow dictionary 'var_deleted' callback to examine the deleted var., Ben Pfaff, 2012/04/15
- [PATCH 09/11] psppire-dict: Make auto_generate_var_name() public, and rename., Ben Pfaff, 2012/04/15
- [PATCH 10/11] psppire-dict: Return new var from psppire_dict_insert_variable()., Ben Pfaff, 2012/04/15
- [PATCH 11/11] psppire-dict: Better validate idx arg in psppire_dict_get_variable()., Ben Pfaff, 2012/04/15
- Re: [PATCH 00/11] first batch of psppsheet changes, John Darrington, 2012/04/16
- Re: [PATCH 00/11] first batch of psppsheet changes,
Ben Pfaff <=