test-book-merge fails in current HEAD

[Derek Atkins]

Neil Williams <linux at codehelp.co.uk> writes:

> Fixed. I fixed a separate bug in qof_book_merge and didn't patch the test 
> case, sorry. I've added a note to explain:

Thanks.

>  /* In this test rule_loop, any lines beginning with do_test() can be removed
>  from a working rule_loop routine. It would be wise to still use some of the
>  more obvious checks, e.g. that an entity or rule exists before querying the 
>  parameters.
>  
>  Take particular care with MERGE_NEW - targetEnt is always NULL until the
>  Commit.
>  Do not attempt to use param_getfcn on targetEnt in the loop called by 
>  qof_book_mergeRuleForeach(rule_loop, MERGE_NEW);
>
> This is because the target book cannot be altered until the Commit, so because 
> this entity is new, it doesn't have a target in the book yet - the target 
> entity in the rule is null. At Commit, a new entity is created using the 
> QofObject create: call which initialises it to the defaults. qof_book_merge 
> then updates the new entity with the import data.
>
> I can't do a lot more, it's down to the developer of the calling process to 
> not call param_getfcn on a NULL entity.

True.  That's out of your hands, and that's ok.  It's called API
semantics and all APIs have them.  You can't protect yourself against
the user of your API breaking your semantics.  That's why you need to
document them to let the user know.  (read: this note also belongs in
the callback docs).

>> Can you please look into this as a high priority?
>
> Patch sent.

Thanks.  It's been applied.  However, wouldn't a better patch be:

  if (rule->mergeResult != MERGE_NEW)
     do_test((rule->targetEnt != NULL), "...");
  else
     do_test((rule->targetEnt == NULL), "...");

Are there cases where targetEnt != NULL when mergeResult == MERGE_NEW?

-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