[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:34:56 -0700 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) |
Ben Pfaff <address@hidden> writes:
> 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.
I don't see a reason to avoid g_return_val_if_fail here so I've
changed the patch to do that instead. Thanks again.
- [PATCH 02/11] psppire-data-store: Use PSPPIRE namespace instead of GTK+'s., (continued)
- [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