Re: [PATCH 01/13] psppire-dict: g_return_if_fail for idx in psppire_dict_get_variable().

[John Darrington]

On Fri, Apr 20, 2012 at 09:24:41PM -0700, Ben Pfaff wrote:
     Ben Pfaff <address@hidden> writes:
     
     > John Darrington <address@hidden> writes:
     >
     >> On Tue, Apr 17, 2012 at 09:13:34PM -0700, Ben Pfaff wrote:
     >>      John Darrington <address@hidden> writes:
     >>      
     >>      >      +  g_return_val_if_fail (idx < 0 || dict_get_var_cnt 
(d->dict) <= idx, NULL);
     >>      >
     >>      > Personally I prefer assertions to be written in their separate 
components rather than ored 
     >>      > together, like this:
     >>      >
     >>      >  g_return_val_if_fail (idx < 0, NULL);
     >>      >  g_return_val_if_fail (dict_get_var_cnt (d->dict) <= idx, NULL);
     >>      >    
     >>      > That way, when the condition fails, it's easier to find out 
which part of the condition is failing.
     >>      
     >>      Fair enough.  I made that change and pushed this commit.
     >>      
     >> This seems to be causing the GUI to crash.
     >
     > Argh.  I've reverted it for the moment, not having time tonight
     > to fix it properly.  Sorry about that.
     
     OK, so I fixed it up to actually be correct (idx >= 0 instead of
     idx < 0), which leads to an immediate:
             ** CRITICAL **: psppire_dict_get_variable: assertion `idx >= 0' 
failed
     upon PSPPIRE startup, with the following backtrace:
     
     #0  0xb7fe2424 in __kernel_vsyscall ()
     #1  0xb725b781 in *__GI_raise (sig=6)
         at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
     #2  0xb725ebb2 in *__GI_abort () at abort.c:92
     #3  0xb7636c0e in IA__g_logv (log_domain=0x0, 
log_level=G_LOG_LEVEL_CRITICAL, 
         format=0xb767b855 "%s: assertion `%s' failed", 
         args1=0xbfffda1c 
"\023\067\v\b\357\037\v\b\200e\377\267\020\217\026\b\020nc\267\250\337\026\b\377\377\377\377X\332\377\277,\343\a\b")
         at 
/build/buildd-glib2.0_2.24.2-1-i386-AScyie/glib2.0-2.24.2/glib/gmessages.c:549
     #4  0xb7636c36 in IA__g_log (log_domain=0x0, 
log_level=G_LOG_LEVEL_CRITICAL, 
         format=0xb767b855 "%s: assertion `%s' failed")
         at 
/build/buildd-glib2.0_2.24.2-1-i386-AScyie/glib2.0-2.24.2/glib/gmessages.c:569
     #5  0xb7636e6b in IA__g_return_if_fail_warning (log_domain=0x0, 
         pretty_function=0x80b3713 "psppire_dict_get_variable", 
         expression=0x80b1fef "idx >= 0")
         at 
/build/buildd-glib2.0_2.24.2-1-i386-AScyie/glib2.0-2.24.2/glib/gmessages.c:584
     #6  0x0807e32c in psppire_dict_get_variable (d=0x816dfa8, idx=-1)
         at ../src/ui/gui/psppire-dict.c:473
     #7  0x0807727f in get_column_button_label (model=0x8196600, col=-1)
         at ../src/ui/gui/psppire-data-store.c:798
     #8  0x0809c61c in psppire_sheet_model_get_column_button (model=0x8196600, 
         col=-1) at ../src/ui/gui/sheet/psppire-sheetmodel.c:483
     #9  0x080a2227 in draw_column_title_buttons_range (sheet=0x815f4d0, 
first=-1, 
         last=<value optimized out>) at ../lib/gtk-contrib/psppire-sheet.c:4245
     #10 0x080a2492 in draw_column_title_buttons (sheet=0x815f4d0)
         at ../lib/gtk-contrib/psppire-sheet.c:3902
     #11 0x080a59bf in psppire_sheet_map (widget=0x815f4d0)
         at ../lib/gtk-contrib/psppire-sheet.c:2024
     
     In other words, it's a bug in gtksheet.  
I feared there might be.

     I'm not enthusiastic
     about fixing that, since I'm trying to replace gtksheet at the
     moment, so I'm inclined to just drop this commit for now.
     
Sounds fine to me.

-- 
PGP Public key ID: 1024D/2DE827B3 
fingerprint = 8797 A26D 0854 2EAB 0285  A290 8A67 719C 2DE8 27B3
See http://keys.gnupg.net or any PGP keyserver for public key.

signature.asc
Description: Digital signature