howard@cyb-eng.UUCP (Howard Johnson) (11/15/85)
A fix to the following 'test' program still has a slight bug. As a matter of design, adding a simple function removes all kinds of nasty portability issues (i.e., sizes of pointers, paramter passing, etc.). >> struct node { int op; struct node *left, *right; double val; } >> *lp, *rp, *p, *makenode(); >> double value; /* constant value from lex */ >> ... >> p = makenode( 'x', lp, rp ); /* create multiply node */ >> ... >> p = makenode( 'k', value ); /* create real constant node */ >> ... >> struct node *makenode( op, arga, argb ) >> int op; struct node *arga, *argb; >> { extern char *malloc(); >> ... >> case 'k': /* real constant */ >> new->val = *(double *)&arga; /* XXX */ >> break; >> } >> return new; >> } >> >> The code failed at point "XXX". The exercise for the >> student is to figure out precisely WHY it fails ...... The attempted fix needs its indirection: > p = makenode ('k', &value ); ... new->val = *((double *)arga); /* you forgot the indirection */ Better! struct node * makeconst(value) double value; { register struct node *new; if ((new = (struct node *) malloc(sizeof *new)) != NULL) { new->op = 'k'; new->val = value; } return(new); } -- ..!{seismo,topaz,mordor,harvard,gatech,nbires,ihnp4}!ut-sally!cyb-eng!howard (ordered best to worst); also ..!ut-ngp!cyb-eng!howard +1 512 458 6609
gwyn@BRL.ARPA (VLD/VMB) (11/15/85)
I suppose it's time to post the answer to the puzzle. The posted code suffered from an alignment problem. The actual function parameter `value' (a double) had to be doubleword-aligned (restriction due to target machine architecture), but no such restriction applied to other parameter types, which were all word-aligned. The attempt to coerce the address of the formal (struct node *) parameter to interpret it as the address of a double therefore was foiled by the fact that there was some padding on the stack before the actual double datum. Note that a similar problem could occur on a big-endian machine without any alignment restrictions, if addresses point at the "wrong end" of multi-byte data. There are several possible fixes to the posted code, but Howard Johnson suggested what I also believe to be the best solution, which is to stop trying to multiplex the uses of the makenode() function and instead create separate functions for different node types. Notice that there is a natural tendency when fixing bugs in existing code to tweak what is already being done, rather than replace it with a better approach.