| Message ID | b40b1bd4adff2d389588d9a4710af0399ce68d1e.1473963064.git.marcelo.leitner@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
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
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
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.
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. >
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
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(-)