[PATCH] warning in SCTP

[PATCH] warning in SCTP

[Meelis Roos]

   CC [M]  net/sctp/sm_make_chunk.o
net/sctp/sm_make_chunk.c: In function 'sctp_unpack_cookie':
net/sctp/sm_make_chunk.c:1358: warning: initialization discards qualifiers from pointer target type

The reason is that sctp_unpack_cookie() takes a const struct 
sctp_endpoint and modifies the digest in it (digest being embedded in 
the struct, not a pointer). So sctp_unpack_cookie really does not use 
the argument as const, mark it as such.

Signed-off-by: Meelis Roos <mroos@linux.ee>

diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
index de313de..ef80489 100644
--- a/include/net/sctp/sm.h
+++ b/include/net/sctp/sm.h
@@ -272,7 +272,7 @@ void sctp_generate_heartbeat_event(unsig

  void sctp_ootb_pkt_free(struct sctp_packet *);

-struct sctp_association *sctp_unpack_cookie(const struct sctp_endpoint *,
+struct sctp_association *sctp_unpack_cookie(struct sctp_endpoint *,
  				       const struct sctp_association *,
  				       struct sctp_chunk *,
  				       gfp_t gfp, int *err,
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 507dff7..3d41a9c 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1346,7 +1346,7 @@ nodata:

  /* Unpack the cookie from COOKIE ECHO chunk, recreating the association.  */
  struct sctp_association *sctp_unpack_cookie(
-	const struct sctp_endpoint *ep,
+	struct sctp_endpoint *ep,
  	const struct sctp_association *asoc,
  	struct sctp_chunk *chunk, gfp_t gfp,
  	int *error, struct sctp_chunk **errp)

-- 
Meelis Roos (mroos@linux.ee)

Re: [PATCH] warning in SCTP

[David Miller]

From: Meelis Roos <mroos@linux.ee>
Date: Thu, 2 Nov 2006 09:29:16 +0200 (EET)

>    CC [M]  net/sctp/sm_make_chunk.o
> net/sctp/sm_make_chunk.c: In function 'sctp_unpack_cookie':
> net/sctp/sm_make_chunk.c:1358: warning: initialization discards qualifiers from pointer target type
> 
> The reason is that sctp_unpack_cookie() takes a const struct 
> sctp_endpoint and modifies the digest in it (digest being embedded in 
> the struct, not a pointer). So sctp_unpack_cookie really does not use 
> the argument as const, mark it as such.
> 
> Signed-off-by: Meelis Roos <mroos@linux.ee>

Applied, but like your other patch I had to apply it by hand.

Something in your email client adds extra spaces to the patch,
for example:

> diff --git a/include/net/sctp/sm.h b/include/net/sctp/sm.h
> index de313de..ef80489 100644
> --- a/include/net/sctp/sm.h
> +++ b/include/net/sctp/sm.h
> @@ -272,7 +272,7 @@ void sctp_generate_heartbeat_event(unsig
> 
>   void sctp_ootb_pkt_free(struct sctp_packet *);
> 

There should only be one space before the "void" in the patch,
your email client (or something else) put another space there.

Also, your email client likes to turn lines containing only
spaces into empty lines, which also corrupts the patch.

Re: [PATCH] warning in SCTP

[Meelis Roos]

> There should only be one space before the "void" in the patch,
> your email client (or something else) put another space there.
>
> Also, your email client likes to turn lines containing only
> spaces into empty lines, which also corrupts the patch.

cg-diff > file
and in pine 4.61 ^R read a file. So I guess it's pine misbehaving... 
Have not heard complaints before but will try to remember to use mutt 
next time.

-- 
Meelis Roos (mroos@linux.ee)

Re: [PATCH] warning in SCTP

[Oleg Verych]

On 2006-11-02, Meelis Roos wrote:
>> There should only be one space before the "void" in the patch,
>> your email client (or something else) put another space there.
>>
>> Also, your email client likes to turn lines containing only
>> spaces into empty lines, which also corrupts the patch.
>
> cg-diff > file
> and in pine 4.61 ^R read a file. So I guess it's pine misbehaving... 
> Have not heard complaints before but will try to remember to use mutt 
> next time.

(quilt || git) mbox -> formail (procmail package), no MUAs, job's done.
____

Re: [PATCH] warning in SCTP

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 02 Nov 2006 00:16:43 -0800 (PST)

> From: Meelis Roos <mroos@linux.ee>
> Date: Thu, 2 Nov 2006 09:29:16 +0200 (EET)
> 
> >    CC [M]  net/sctp/sm_make_chunk.o
> > net/sctp/sm_make_chunk.c: In function 'sctp_unpack_cookie':
> > net/sctp/sm_make_chunk.c:1358: warning: initialization discards qualifiers from pointer target type
> > 
> > The reason is that sctp_unpack_cookie() takes a const struct 
> > sctp_endpoint and modifies the digest in it (digest being embedded in 
> > the struct, not a pointer). So sctp_unpack_cookie really does not use 
> > the argument as const, mark it as such.
> > 
> > Signed-off-by: Meelis Roos <mroos@linux.ee>
> 
> Applied,

Actually, I'm backing this one out, it creates new warnings because
callers of this function pass in a "const" pointer.

net/sctp/sm_statefuns.c: In function ^[$,1rx^[(Bsctp_sf_do_5_1D_ce^[$,1ry^[(B:
net/sctp/sm_statefuns.c:630: warning: passing argument 1 of ^[$,1rx^[(Bsctp_unpack_cookie^[$,1ry^[(B discards qualifiers from pointer target type
net/sctp/sm_statefuns.c: In function ^[$,1rx^[(Bsctp_sf_do_5_2_4_dupcook^[$,1ry^[(B:
net/sctp/sm_statefuns.c:1884: warning: passing argument 1 of ^[$,1rx^[(Bsctp_unpack_cookie^[$,1ry^[(B discards qualifiers from pointer target type

Please test the complete build when doing warning fixes like
this in the future, thanks.

Re: [PATCH] warning in SCTP

[Meelis Roos]

> Actually, I'm backing this one out, it creates new warnings because
> callers of this function pass in a "const" pointer.

OK, fair enough. Will have a look.

> Please test the complete build when doing warning fixes like
> this in the future, thanks.

Sure. Evenso, I did test full make after the single compile was fine 
(still have it in my scrollback so here it is). So did I do anything 
wrong here or do we have some dependencies broken somewhere or what?
Now that I read the transript again I can see that despite modifying 
include/net/sctp/sm.h nothing was really recompiled - should have 
noticed.

mroos@rhn:~/compile/linux-2.6$ vi net/sctp/sm_make_chunk.c
mroos@rhn:~/compile/linux-2.6$ !make
make net/sctp/sm_make_chunk.o
   CHK     include/linux/version.h
   CHK     include/linux/utsrelease.h
   CC      net/sctp/sm_make_chunk.o
net/sctp/sm_make_chunk.c:1353: error: conflicting types for 'sctp_unpack_cookie'
include/net/sctp/sm.h:279: error: previous declaration of 'sctp_unpack_cookie' was here
make[1]: *** [net/sctp/sm_make_chunk.o] Error 1
make: *** [net/sctp/sm_make_chunk.o] Error 2
mroos@rhn:~/compile/linux-2.6$ vi include/net/sctp/sm.h
mroos@rhn:~/compile/linux-2.6$ make net/sctp/sm_make_chunk.o
   CHK     include/linux/version.h
   CHK     include/linux/utsrelease.h
   CC      net/sctp/sm_make_chunk.o
mroos@rhn:~/compile/linux-2.6$ make
   CHK     include/linux/version.h
   CHK     include/linux/utsrelease.h
   CHK     include/linux/compile.h
   MODPOST vmlinux
Kernel: arch/i386/boot/bzImage is ready  (#274)
   Building modules, stage 2.
   MODPOST 511 modules
mroos@rhn:~/compile/linux-2.6$ cg-diff  > ~/sctppatch


-- 
Meelis Roos (mroos@linux.ee)

Re: [PATCH] warning in SCTP

[Meelis Roos]

> Sure. Evenso, I did test full make after the single compile was fine (still 
> have it in my scrollback so here it is). So did I do anything wrong here or 
> do we have some dependencies broken somewhere or what?

Never mind - I discovered the warning on one box with SCTP enabled in 
config and tried the fix on a faster machine that unfortunately had SCTP 
disabled.

-- 
Meelis Roos (mroos@ut.ee)      http://www.cs.ut.ee/~mroos/

Re: [PATCH] warning in SCTP

[Meelis Roos]

> Actually, I'm backing this one out, it creates new warnings because
> callers of this function pass in a "const" pointer.

Yes, it now seems it's not so simple. Marking it non-const there would 
mark the it non-const in the whole family of sctp_state_fn_t and I'm not 
sure that's the best thing to do. I guess the maintainer has better 
bases for deciding what to do about it.

-- 
Meelis Roos (mroos@linux.ee)

Re: [PATCH] warning in SCTP

[Vlad Yasevich]

[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]

Meelis Roos wrote:
>> Actually, I'm backing this one out, it creates new warnings because
>> callers of this function pass in a "const" pointer.
> 
> Yes, it now seems it's not so simple. Marking it non-const there would
> mark the it non-const in the whole family of sctp_state_fn_t and I'm not
> sure that's the best thing to do. I guess the maintainer has better
> bases for deciding what to do about it.
> 

An alternate solution would be to make the digest a pointer, allocate
it in sctp_endpoint_init() and free it in sctp_endpoint_destroy().

I guess I should have originally done it this way...


  CC [M]  net/sctp/sm_make_chunk.o
net/sctp/sm_make_chunk.c: In function 'sctp_unpack_cookie':
net/sctp/sm_make_chunk.c:1358: warning: initialization discards qualifiers from pointer target type

The reason is that sctp_unpack_cookie() takes a const struct sctp_endpoint and modifies the digest in it
(digest being embedded in the struct, not a pointer).  Make digest a pointer to fix this warning.

Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com> 




[-- Attachment #2: digest.diff --]
[-- Type: text/x-patch, Size: 1184 bytes --]

diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index c6d93bb..5596f5d 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1270,7 +1270,7 @@ struct sctp_endpoint {
  	 * 	    this here so we pre-allocate this once and can re-use
  	 * 	    on every receive.
  	 */
- 	__u8 digest[SCTP_SIGNATURE_SIZE];
+ 	__u8 *digest;
  
 	/* sendbuf acct. policy.	*/
 	__u32 sndbuf_policy;
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 35c49ff..4576933 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -72,6 +72,10 @@ static struct sctp_endpoint *sctp_endpoi
 {
 	memset(ep, 0, sizeof(struct sctp_endpoint));
 
+	ep->digest = kzalloc(SCTP_SIGNATURE_SIZE, gfp);
+	if (!ep->digest)
+		return NULL;
+
 	/* Initialize the base structure. */
 	/* What type of endpoint are we?  */
 	ep->base.type = SCTP_EP_TYPE_SOCKET;
@@ -175,6 +180,9 @@ static void sctp_endpoint_destroy(struct
 	/* Free up the HMAC transform. */
 	crypto_free_hash(sctp_sk(ep->base.sk)->hmac);
 
+	/* Free the digest buffer */
+	kfree(ep->digest);
+
 	/* Cleanup. */
 	sctp_inq_free(&ep->base.inqueue);
 	sctp_bind_addr_free(&ep->base.bind_addr);

Re: [PATCH] warning in SCTP

[Sridhar Samudrala]

On Thu, 2006-11-02 at 11:09 -0500, Vlad Yasevich wrote:
> Meelis Roos wrote:
> >> Actually, I'm backing this one out, it creates new warnings because
> >> callers of this function pass in a "const" pointer.
> > 
> > Yes, it now seems it's not so simple. Marking it non-const there would
> > mark the it non-const in the whole family of sctp_state_fn_t and I'm not
> > sure that's the best thing to do. I guess the maintainer has better
> > bases for deciding what to do about it.
> > 
> 
> An alternate solution would be to make the digest a pointer, allocate
> it in sctp_endpoint_init() and free it in sctp_endpoint_destroy().

I agree that this is a better solution.

Acked-by: Sridhar Samudrala <sri@us.ibm.com>

Thanks
Sridhar

> 
> I guess I should have originally done it this way...
> 
> 
>   CC [M]  net/sctp/sm_make_chunk.o
> net/sctp/sm_make_chunk.c: In function 'sctp_unpack_cookie':
> net/sctp/sm_make_chunk.c:1358: warning: initialization discards qualifiers from pointer target type
> 
> The reason is that sctp_unpack_cookie() takes a const struct sctp_endpoint and modifies the digest in it
> (digest being embedded in the struct, not a pointer).  Make digest a pointer to fix this warning.
> 
> Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com> 
> 
> 
> 

Re: [PATCH] warning in SCTP

[David Miller]

From: Sridhar Samudrala <sri@us.ibm.com>
Date: Thu, 02 Nov 2006 10:29:49 -0800

> On Thu, 2006-11-02 at 11:09 -0500, Vlad Yasevich wrote:
> > Meelis Roos wrote:
> > >> Actually, I'm backing this one out, it creates new warnings because
> > >> callers of this function pass in a "const" pointer.
> > > 
> > > Yes, it now seems it's not so simple. Marking it non-const there would
> > > mark the it non-const in the whole family of sctp_state_fn_t and I'm not
> > > sure that's the best thing to do. I guess the maintainer has better
> > > bases for deciding what to do about it.
> > > 
> > 
> > An alternate solution would be to make the digest a pointer, allocate
> > it in sctp_endpoint_init() and free it in sctp_endpoint_destroy().
> 
> I agree that this is a better solution.
> 
> Acked-by: Sridhar Samudrala <sri@us.ibm.com>

Applied to net-2.6.20, thanks everyone.