Re: [PATCH] pk_equal_val_p function and tests.

[Jose E. Marchesi]

>>>  void
>>>  pvm_allocate_struct_attrs (pvm_val nfields,
>>>                             pvm_val **fnames, pvm_val **ftypes)
>>> @@ -1103,30 +1390,25 @@ pvm_type_equal (pvm_val type1, pvm_val type2)
>>>        {
>>>          size_t t1_size = PVM_VAL_ULONG (PVM_VAL_TYP_I_SIZE (type1));
>>>          size_t t2_size = PVM_VAL_ULONG (PVM_VAL_TYP_I_SIZE (type2));
>>> -        uint32_t t1_signed = PVM_VAL_INT (PVM_VAL_TYP_I_SIGNED_P
>>> (type1));
>>> -        uint32_t t2_signed = PVM_VAL_INT (PVM_VAL_TYP_I_SIGNED_P
>>> (type2));
>>> +        int32_t t1_signed = PVM_VAL_INT (PVM_VAL_TYP_I_SIGNED_P
>>> (type1));
>>> +        int32_t t2_signed = PVM_VAL_INT (PVM_VAL_TYP_I_SIGNED_P
>>> (type2));
>> Does this belong to a separated patch?
>>
>
> Yes, kinda. It wasn't really a bug there even if its uint32_t instead of
> int32_t. My eye fell on it, I corrected it. I believed it was a change
> not-worthy of a whole patch.
>
> However, it was my bad I did not document this change.
>
> Is it ok if I apply it again on the next version of this patch?

It is better to have separated patchs for separated things.  This is for
many reasons.

>>>
>>>          return (t1_size == t2_size && t1_signed == t2_signed);
>>> -        break;
>>
>> Huh?
>>
>
> Same here, having a break statement after return did not seemed logical to
> me, but again, didn't think it was worthy of a whole patch.
>
> Sorry if that disturbed you.

No problem!  But please use separated commits for this.  git commits are
cheap! :)

>
>>> +/* Compare 2 PVM values.
>>
>> Two
>>
>
> Done.
>
>> In the tests, I would use:
>>
>> <prologue>
>> ##
>> <expr1>
>> ##
>> <expr2>
>>
>> Instead of having two consecutive expressions.
>>
>
> OK, will fix it.
>
> Thanks for your comments, I will send the next version of this patch this
> evening.

Thanks.