I don’t get the point.

Netlink sequence numbers are opaque and are needed only to correlate 
request/response. Netlink sequence numbers are build by this macro 
(module/owlib/src/include/ow_w1.h):

#define MAKE_NL_SEQ( bus, seq )  ((uint32_t)(( ((bus) & 0xFFFF) << 16 ) | 
((seq) & 0xFFFF)))

This does not mean that one should have 0 < seq <= 0xFFFF, provided that when 
you parse the response you properly mask both the response netlink sequence 
number and the owlib seq (which originated the request) to check only the low 4 
bytes, i.e.

NL_SEQ(netlink response sequence number) == NL_SEQ(owlib internal sequence 
number seq)

IMHO concrete evidence should be provided that there is a point in the source 
where this masking is not done properly, and that is the bug to be corrected. 
Fudging the sequence number in order to avoid a bug  surfacing in another point 
of the code is nooot good.

S.

> On 13 Nov 2016, at 11:10, Jan Kandziora <j...@gmx.de> wrote:
> 
> Am 13.11.2016 um 00:53 schrieb Enrico Hoepfner:
>> Hi Jan,
>> 
>> Thank you for the fast answer!
>> I dont understand exacly , which patch and test from Paul you mean.
>> Maybe I have take the diff in the wrong direction???? sorry for that!
>> 
>> this are the new lines
>> 
>> <               // seq = ++in->master.w1.seq ;
>> <               // seq should not be zero or > 0xFFFF
>> <               seq = NL_SEQ(++in->master.w1.seq);
>> <               if(seq == 0) {
>> <                 seq = NL_SEQ(++in->master.w1.seq);
>> <                 LEVEL_DEBUG("NETLINK sequence number overrun");
>> <               }
>> 
>> 
>> this is what should be replaced
>> 
>>>              seq = ++in->master.w1.seq ;
>> 
> Aaaahhhh, that's why diff -u is preferred.
> 
> 
>> 
>> 
>> diff -u ow_w1_send.c.orig ow_w1_send.c
>> --- ow_w1_send.c.orig    2016-02-04 21:09:53.000000000 +0100
>> +++ ow_w1_send.c        2016-11-08 20:55:51.351153464 +0100
>> @@ -68,7 +68,13 @@
>>         } else {
>>                 // w1 subsidiary bus
>>                 // this bus is locked
>> -               seq = ++in->master.w1.seq ;
>> +               // seq = ++in->master.w1.seq ;
>> +               // seq should not be zero or > 0xFFFF
>> +               seq = NL_SEQ(++in->master.w1.seq);
>> +               if(seq == 0) {
>> +                 seq = NL_SEQ(++in->master.w1.seq);
>> +                 LEVEL_DEBUG("NETLINK sequence number overrun");
>> +               }
>>                 bus = in->master.w1.id;
>>         }
>> 
>> 
> Could you explain what the patch does? Two sentences?
> 
> Do you think the DEBUG message is necessary? If it's a normal condition
> which can happen anytime, it's likely nothing to be debugged.
> 
> Kind regards
> 
>       Jan
> 
> ------------------------------------------------------------------------------
> Developer Access Program for Intel Xeon Phi Processors
> Access to Intel Xeon Phi processor-based developer platforms.
> With one year of Intel Parallel Studio XE.
> Training and support from Colfax.
> Order your platform today. http://sdm.link/xeonphi
> _______________________________________________
> Owfs-developers mailing list
> Owfs-developers@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/owfs-developers

------------------------------------------------------------------------------
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
_______________________________________________
Owfs-developers mailing list
Owfs-developers@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/owfs-developers

Reply via email to