[PATCH next] sctp: make use of WORD_TRUNC macro

[PATCH next] sctp: make use of WORD_TRUNC macro

[Marcelo Ricardo Leitner]

No functional change. Just to avoid the usage of '&~3'.
Also break the line to make it easier to read.

Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/chunk.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index a55e54738b81ff8cf9cd711cf5fc466ac71374c0..adae4a41ca2078cfee387631f76e5cb768c2269c 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -182,9 +182,10 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
 	/* This is the biggest possible DATA chunk that can fit into
 	 * the packet
 	 */
-	max_data = (asoc->pathmtu -
-		sctp_sk(asoc->base.sk)->pf->af->net_header_len -
-		sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk)) & ~3;
+	max_data = asoc->pathmtu -
+		   sctp_sk(asoc->base.sk)->pf->af->net_header_len -
+		   sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk);
+	max_data = WORD_TRUNC(max_data);
 
 	max = asoc->frag_point;
 	/* If the the peer requested that we authenticate DATA chunks
-- 
2.7.4

RE: [PATCH next] sctp: make use of WORD_TRUNC macro

[David Laight]

From: Marcelo Ricardo Leitner
> Sent: 15 September 2016 19:13
> No functional change. Just to avoid the usage of '&~3'.
...
> -	max_data = (asoc->pathmtu -
> -		sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> -		sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk)) & ~3;
> +	max_data = asoc->pathmtu -
> +		   sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> +		   sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk);
> +	max_data = WORD_TRUNC(max_data);

Hmmm....
Am I the only person who understands immediately what & ~3 does
but would have to grovel through the headers to find exactly what
WORD_TRUNC() does.

How big is a 'WORD' anyway??

	David

Re: [PATCH next] sctp: make use of WORD_TRUNC macro

['Marcelo Ricardo Leitner']

On Fri, Sep 16, 2016 at 09:51:56AM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 15 September 2016 19:13
> > No functional change. Just to avoid the usage of '&~3'.
> ...
> > -	max_data = (asoc->pathmtu -
> > -		sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> > -		sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk)) & ~3;
> > +	max_data = asoc->pathmtu -
> > +		   sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> > +		   sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk);
> > +	max_data = WORD_TRUNC(max_data);
> 
> Hmmm....
> Am I the only person who understands immediately what & ~3 does
> but would have to grovel through the headers to find exactly what
> WORD_TRUNC() does.
> 

ctags & cia can help you with that. :)

> How big is a 'WORD' anyway??

That's pretty much one of the reasons for using the macro, to make sure
it is correctly adapted to some arch if necessary. (even though it's
not necessary in this case)

  Marcelo

Re: [PATCH next] sctp: make use of WORD_TRUNC macro

[David Miller]

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 15 Sep 2016 15:12:30 -0300

> No functional change. Just to avoid the usage of '&~3'.
> Also break the line to make it easier to read.

You're reply later in this thread:

	"to make sure it is correctly adapted to some arch if
	necessary. (even though it's not necessary in this case)"

is inconsistent with your commit log message.

If you think that the word size might possibly be different
on a given arch, then this is in fact a functional change.

This patch just adds ambiguity.  Whereas the existing code is explicit
about "multiple of 4" and there can be no confusion.

I'm not applying this, sorry.

Re: [PATCH next] sctp: make use of WORD_TRUNC macro

[Marcelo Ricardo Leitner]

Em 18-09-2016 23:06, David Miller escreveu:
> From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> Date: Thu, 15 Sep 2016 15:12:30 -0300
>
>> No functional change. Just to avoid the usage of '&~3'.
>> Also break the line to make it easier to read.
>
> You're reply later in this thread:
>
> 	"to make sure it is correctly adapted to some arch if
> 	necessary. (even though it's not necessary in this case)"
>
> is inconsistent with your commit log message.
>
> If you think that the word size might possibly be different
> on a given arch, then this is in fact a functional change.
>

Alright, that was badly worded, sorry. I meant not about the macro in 
specific but in a more general way, as in to not use magic hardcoded 
values, just that.

> This patch just adds ambiguity.  Whereas the existing code is explicit
> about "multiple of 4" and there can be no confusion.
>

On the other hand, it brings the code closer to a standard. This is the 
one but last occurrence of '~3' throughout sctp code. There is only one 
other spot left. All of them are using WORD_ROUND or WORD_TRUNC macros 
already.

We can rename the macros, I agree they sound confusing. Proposing 
SCTP_ALIGN4 and SCTP_TRUNC4. Does that sound better? Then I'll send a 
patchset renaming and updating all remaining places.

> I'm not applying this, sorry.
>