On Fri, Sep 16, 2016 at 02:17:15PM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 16 September 2016 14:45
> > To: David Laight
> > Cc: 'Neil Horman'; linux-sctp@xxxxxxxxxxxxxxx
> > Subject: Re: [PATCH net] sctp: fix SSN comparision
> >
> > On Fri, Sep 16, 2016 at 10:40:47AM -0300, Marcelo Ricardo Leitner wrote:
> > > On Fri, Sep 16, 2016 at 01:33:47PM +0000, David Laight wrote:
> > > > > > -static inline int ADDIP_SERIAL_gte(__u16 s, __u16 t)
> > > > > > +static inline int ADDIP_SERIAL_gte(__u32 s, __u32 t)
> > > > > > {
> > > > > > return ((s) == (t)) || (((t) - (s)) & ADDIP_SERIAL_SIGN_BIT);
> > > > > > }
> > > >
> > > > Does gcc manage to compile that to (s32)((t) - (s)) <= 0 ?
> > >
> > > I have no idea but I have a patch in the works for updating it to this
> > > form, to be more like time_after(). Will probably post it by next week.
> >
> > Just for reference, this should be it. Not realy tested yet.
> >
> > commit b52575533dfefa1908c7bbe35bde0fa8359dd1a3
> > Author: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
> > Date: Tue Aug 9 14:45:35 2016 -0300
> >
> > sctp: improve how SSN, TSN and ASCONF serial are compared
> >
> > Make it similar to time_before() macros instead.
> >
> > This patch also removes SSN_lte as it is not used.
> ...
> > +#define TSN_lte(a,b) \
> > + (typecheck(__u32, a) && \
> > + typecheck(__u32, b) && \
> > + ((__s32)((a) - (b)) <= 0))
> ...
> > + for (i = 0; i < blocks; ++i) {
> > + if (TSN_lte((__u32)ntohs(frags[i].gab.start), (__u32)gap) &&
> > + TSN_lte((__u32)gap, (__u32)ntohs(frags[i].gab.end)))
>
> Those casts look decidedly dubious, messy at best.
> I'm pretty sure you're not managing modulo 2^16 arithmetic either.
Agreed.
Just to highlight, this is an issue today already, as they are promoted
to u32 on the function call.
> I think you want:
> #define TSN_lte(a, b) ((__s16)((a) - (b)) <= 0)
> If a and b are 16bit the subtract is always well defined.
No because it's also used in other places and with actual 32bit vars.
This was the only place that required such casts.
Thanks,
Marcelo
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
