poke-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] pk_equal_val_p function and tests.


From: Konstantinos Chasialis
Subject: Re: [PATCH] pk_equal_val_p function and tests.
Date: Fri, 7 Aug 2020 16:46:04 +0300
User-agent: SquirrelMail/1.4.23 [email.uoa.gr]

> Two closure values are always not equal.

OK, removed.

>>  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?

>>
>>          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.

>> +/* 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.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]