* [PATCH] dccp: ensure gap does not become unsigned -1 @ 2009-03-03 20:24 Roel Kluin 2009-03-04 11:45 ` gerrit 0 siblings, 1 reply; 3+ messages in thread From: Roel Kluin @ 2009-03-03 20:24 UTC (permalink / raw) To: dccp Is this maybe necessary? ------------------------------>8-------------8<--------------------------------- if packets is 0, becomes -1, but since gap is unsigned, the test 'gap > 0' does not work. Signed-off-by: Roel Kluin <roel.kluin@gmail.com> --- diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c index 01e4d39..7c61bc0 100644 --- a/net/dccp/ackvec.c +++ b/net/dccp/ackvec.c @@ -207,7 +207,11 @@ static inline int dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av, if (av->av_vec_len + packets > DCCP_MAX_ACKVEC_LEN) return -ENOBUFS; - gap = packets - 1; + if (packets > 0) + gap = packets - 1; + else + gap = 0; + new_head = av->av_buf_head - packets; if (new_head < 0) { ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] dccp: ensure gap does not become unsigned -1 2009-03-03 20:24 [PATCH] dccp: ensure gap does not become unsigned -1 Roel Kluin @ 2009-03-04 11:45 ` gerrit 0 siblings, 0 replies; 3+ messages in thread From: gerrit @ 2009-03-04 11:45 UTC (permalink / raw) To: dccp [added missing copy to netdev@vger] > Is this maybe necessary? > ------------------------------>8-------------8<--------------------------------- > if packets is 0, becomes -1, but since gap is unsigned, the test 'gap > 0' > does not work. > Thank you for looking through. It is not necessary, see below. > --- a/net/dccp/ackvec.c > +++ b/net/dccp/ackvec.c > @@ -207,7 +207,11 @@ static inline int > dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av, > if (av->av_vec_len + packets > DCCP_MAX_ACKVEC_LEN) > return -ENOBUFS; > > - gap = packets - 1; > + if (packets > 0) Why 'gap < 0' <=> packets < 0 can not happen: /* * If several packets are missing, the HC-Receiver may prefer to enter multiple * bytes with run length 0, rather than a single byte with a larger run length; * this simplifies table updates if one of the missing packets arrives. */ static inline int dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av, const unsigned int packets, const unsigned char state) This is a static routine which is called in one place only in net/dccp/ackvec.c, in dccp_ackvec_add(): //... } else if (after48(ackno, av->av_buf_ackno)) { const u64 delta = dccp_delta_seqno(av->av_buf_ackno, ackno); /* * Look if the state of this packet is the same as the * previous ackno and if so if we can bump the head len. */ if (delta = 1 && dccp_ackvec_state(av, av->av_buf_head) = state && dccp_ackvec_len(av, av->av_buf_head) < DCCP_ACKVEC_LEN_MASK) av->av_buf[av->av_buf_head]++; else if (dccp_ackvec_set_buf_head_state(av, delta, state)) return -ENOBUFS; => We thus have: 1) after48(a, b) => sequence number `a' is `after' b 2) sequence number `a' is `after' `b' => dccp_delta_seqno(b, a) > 0 3) hence delta > 0 4) hence delta - 1 = gap >= 0 5) The subsequent `else' in dccp_ackvec_add() catches the case of !after48(a, b), note the reversed order of arguments to dccp_delta_seqno, so that a positive value is returned: } else { /* * A.1.2. Old Packets * * When a packet with Sequence Number S <= buf_ackno * arrives, the HC-Receiver will scan the table for * the byte corresponding to S. (Indexing structures * could reduce the complexity of this scan.) */ u64 delta = dccp_delta_seqno(ackno, av->av_buf_ackno); => But this is in an entirely different block of code outside the one to which this patch was meant. Hence it is not necessary. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] dccp: ensure gap does not become unsigned -1 @ 2009-03-04 11:45 ` gerrit 0 siblings, 0 replies; 3+ messages in thread From: gerrit @ 2009-03-04 11:45 UTC (permalink / raw) To: Roel Kluin; +Cc: netdev, acme, dccp [added missing copy to netdev@vger] > Is this maybe necessary? > ------------------------------>8-------------8<--------------------------------- > if packets is 0, becomes -1, but since gap is unsigned, the test 'gap > 0' > does not work. > Thank you for looking through. It is not necessary, see below. > --- a/net/dccp/ackvec.c > +++ b/net/dccp/ackvec.c > @@ -207,7 +207,11 @@ static inline int > dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av, > if (av->av_vec_len + packets > DCCP_MAX_ACKVEC_LEN) > return -ENOBUFS; > > - gap = packets - 1; > + if (packets > 0) Why 'gap < 0' <=> packets < 0 can not happen: /* * If several packets are missing, the HC-Receiver may prefer to enter multiple * bytes with run length 0, rather than a single byte with a larger run length; * this simplifies table updates if one of the missing packets arrives. */ static inline int dccp_ackvec_set_buf_head_state(struct dccp_ackvec *av, const unsigned int packets, const unsigned char state) This is a static routine which is called in one place only in net/dccp/ackvec.c, in dccp_ackvec_add(): //... } else if (after48(ackno, av->av_buf_ackno)) { const u64 delta = dccp_delta_seqno(av->av_buf_ackno, ackno); /* * Look if the state of this packet is the same as the * previous ackno and if so if we can bump the head len. */ if (delta == 1 && dccp_ackvec_state(av, av->av_buf_head) == state && dccp_ackvec_len(av, av->av_buf_head) < DCCP_ACKVEC_LEN_MASK) av->av_buf[av->av_buf_head]++; else if (dccp_ackvec_set_buf_head_state(av, delta, state)) return -ENOBUFS; ==> We thus have: 1) after48(a, b) => sequence number `a' is `after' b 2) sequence number `a' is `after' `b' => dccp_delta_seqno(b, a) > 0 3) hence delta > 0 4) hence delta - 1 = gap >= 0 5) The subsequent `else' in dccp_ackvec_add() catches the case of !after48(a, b), note the reversed order of arguments to dccp_delta_seqno, so that a positive value is returned: } else { /* * A.1.2. Old Packets * * When a packet with Sequence Number S <= buf_ackno * arrives, the HC-Receiver will scan the table for * the byte corresponding to S. (Indexing structures * could reduce the complexity of this scan.) */ u64 delta = dccp_delta_seqno(ackno, av->av_buf_ackno); ==> But this is in an entirely different block of code outside the one to which this patch was meant. Hence it is not necessary. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-03-04 11:45 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-03 20:24 [PATCH] dccp: ensure gap does not become unsigned -1 Roel Kluin 2009-03-04 11:45 ` gerrit 2009-03-04 11:45 ` gerrit
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.