Re: Small fix for _equalValue()

Fernando Nasser <fnasser(at)redhat(dot)com> writes:
> *************** _equalValue(Value *a, Value *b)
> *** 1771,1777 ****
> case T_Float:
> case T_String:
> case T_BitString:
> ! return strcmp(a->val.str, b->val.str) == 0;
> default:
> break;
> }
> --- 1771,1780 ----
> case T_Float:
> case T_String:
> case T_BitString:
> ! if ((a->val.str != NULL) && (b->val.str != NULL))
> ! return strcmp(a->val.str, b->val.str) == 0;
> ! else
> ! return a->val.ival == b->val.ival; /* true if both are NULL */
> default:
> break;
> }

Several comments here:

This is not the idiomatic way to do it; there is an equalstr() macro
in equalfuncs.c that does this pushup for you. So "return
equalstr(a->val.str, b->val.str)" would be the appropriate fix.

Possibly a more interesting question, though, is *why* equalValue is
seeing Values with null pointer parts. I cannot think of any good
reason to consider that a legal data structure. Do you know where this
construct is coming from? I'd be inclined to consider the source at
fault, not equalValue.

On the other fixes: as a rule, a field-typing bug in copyfuncs suggests
an equivalent bug over in equalfuncs, and vice versa; as well as
possible errors in readfuncs/outfuncs. Did you look?

regards, tom lane