Patch for bug #103174

[Derek Atkins]

Nigel Titley <nigel at titley.com> writes:

> > Am I missing something in your patch?
> 
> Yes you are. The problem is that the SX formula_cell stuff calls (via an
> intermediate step) 
> 
> gnc_basic_cell_set_value_internal (BasicCell *cell, const char *value)
> 
> with *value pointing at cell->value. The overall effect of this as far
> as gnc_basic_cell_set_value_internal is concerned is to trash the value
> stored in cell->value *before* re-writing back to the same place (the
> g_free() writes memory management stuff in the first few bytes of the
> value). This is what trashes the debit/credit value in the SX register
> entry. Now there are at least three ways to fix this:

Aha..  yes, that does make it more clear...

> 1. Modify the formula cell code to save cell->value in a temporary value
> 
> 2. Add a test in gnc_basic_cell_set_value_internal() to not do the
> assignment in the case where the source and destination are the same
> 
> 3. Store the value in a temporary variable to avoid the overwrite.

Yep, I agree with these options...

> I decided on 2 or 3 because it makes gnc_basic_cell_set_value_internal()
> safer. Whether you prefer a solution based on strategy 2 or 3 depends on
> your mindset. I happen to prefer 3, hence the patch.

I think #2 would be better because it's one fewer malloc/free.  Basically:

   if (cell->value == value)
      return;

It's a faster operation than unconditionally malloc/freeing.  Although
the flip-side of this argument is that the NORMAL case is the
free/malloc, so it's adding a useless test in the normal case.  So I
can go either way.

> Sorry it has taken me so long to reply to this. I wasn't subscribed to
> gnucash-patches. I've been sitting here getting slightly upset that I've
> had no response..... my fault. I eventually decided to check the archive
> and saw your question. 

Strange -- I thought I sent it to you directly as well as to the list.

> I've now subscribed.... if someone could either check in the patch or
> let me know what you would like me to do. This patch fixes 4 extant bugs
> in the SX part of bugzilla.

May I ask your reasoning for choosing #3 over #2?  If we continue down
the road of #2 we should add a comment that explains why.  (If we
choose #2 I think it's quite clear what you're doing).

Thanks!

> All the best
> 
> Nigel
> -- 
> Nigel Titley <nigel at titley.com>

-derek

-- 
       Derek Atkins, SB '93 MIT EE, SM '95 MIT Media Laboratory
       Member, MIT Student Information Processing Board  (SIPB)
       URL: http://web.mit.edu/warlord/    PP-ASEL-IA     N1NWH
       warlord at MIT.EDU                        PGP key available