Adapt CCID-4 according to the latest changes to CCID-3

PATCH In review Leandro 3 participants 5 messages Nov 1, 2007 – Nov 8, 2007

lore

Share & tools

lore

Leandro

author

2007-11-01 00:30 UTC #0 lore raw

PATCH 9/25 Adapt CCID-4 according to the latest changes to CCID-3

[CCID-4] Adapt CCID-4 according to the latest changes to CCID-3

Signed-off-by Leandro Melo de Sales <[email protected]>

Index: leandro.new/net/dccp/ccids/ccid4.c =================================--- leandro.new.orig/net/dccp/ccids/ccid4.c +++ leandro.new/net/dccp/ccids/ccid4.c
@@ -156,7 +156,6 @@ static inline void ccid4_hc_tx_x_header_
*
*/
static void ccid4_hc_tx_update_x(struct sock *sk, ktime_t *stamp)
-
{
struct ccid4_hc_tx_sock *hctx = ccid4_hc_tx_sk(sk); __u64 min_rate = 2 * hctx->ccid4hctx_x_recv;
@@ -939,7 +938,7 @@ static int ccid4_hc_rx_getsockopt(struct

static struct ccid_operations ccid4 = {
.ccid_id = DCCPC_CCID4,
- .ccid_name = "ccid4",
+ .ccid_name = "TCP-Friendly Rate Control (Small-Packet variant)",
.ccid_owner = THIS_MODULE,
.ccid_hc_tx_obj_size = sizeof(struct ccid4_hc_tx_sock),
.ccid_hc_tx_init = ccid4_hc_tx_init,
Index: leandro.new/net/dccp/ccids/ccid4.h =================================--- leandro.new.orig/net/dccp/ccids/ccid4.h +++ leandro.new/net/dccp/ccids/ccid4.h
@@ -128,10 +128,9 @@ struct ccid4_hc_tx_sock {

static inline struct ccid4_hc_tx_sock *ccid4_hc_tx_sk(const struct sock *sk) {
- void *ccid4_tx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
-
- BUG_ON(ccid4_tx_priv = NULL);
- return ccid4_tx_priv;

+    struct ccid4_hc_tx_sock *hctx = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
+    BUG_ON(hctx = NULL);
+    return hctx;

}

/* CCID4 receiver states */
@@ -177,10 +176,9 @@ struct ccid4_hc_rx_sock {

static inline struct ccid4_hc_rx_sock *ccid4_hc_rx_sk(const struct sock *sk) {
- void *ccid4_rx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
-
- BUG_ON(ccid4_rx_priv = NULL);
- return ccid4_rx_priv;

+    struct ccid4_hc_rx_sock *hcrx = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
+    BUG_ON(hcrx = NULL);
+    return hcrx;

}

#endif /* _DCCP_CCID4_H_ */

Tommi Saviranta

2007-11-01 12:32 UTC #1 lore raw

Re: [PATCH 9/25] Adapt CCID-4 according to the latest changes to

Quoted 3 lines · Leandro

[CCID-4] Adapt CCID-4 according to the latest changes to CCID-3
Signed-off-by Leandro Melo de Sales <[email protected]>

Another hit from whitespace police. This isn't really about Leandro's code, but generally about whitespace usage in Linux kernel. Obviously people prefer aligning things:

Review

Quoted 3 lines

(quoted diff) +0 -0

 	.ccid_owner		   = THIS_MODULE,
 	.ccid_hc_tx_obj_size	   = sizeof(struct ccid4_hc_tx_sock),
 	.ccid_hc_tx_init	   = ccid4_hc_tx_init,

Sometimes tabulators are used to fill up most of the gap, followed by spaces (like in this case). Sometimes only spaces are used, even for gaps longer seven characters. Is there a general consensus about this? Do the spaces/tabs just get mixed as the code evolves?

Quoted 19 lines

Index: leandro.new/net/dccp/ccids/ccid4.h =================================> --- leandro.new.orig/net/dccp/ccids/ccid4.h +++ leandro.new/net/dccp/ccids/ccid4.h

(quoted diff) +3 -4

@@ -128,10 +128,9 @@ struct ccid4_hc_tx_sock {
 
 static inline struct ccid4_hc_tx_sock *ccid4_hc_tx_sk(const struct sock *sk)
 {
-	void *ccid4_tx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
-
-	BUG_ON(ccid4_tx_priv = NULL);
-	return ccid4_tx_priv;
+    struct ccid4_hc_tx_sock *hctx = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
+    BUG_ON(hctx = NULL);
+    return hctx;
 }
 
 /* CCID4 receiver states */

@@ -177,10 +176,9 @@ struct ccid4_hc_rx_sock {

Review struct ccid4_hc_rx_sock { · line 182

Quoted 10 lines

(quoted diff) +3 -4

 static inline struct ccid4_hc_rx_sock *ccid4_hc_rx_sk(const struct sock *sk)
 {
-	void *ccid4_rx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
-
-	BUG_ON(ccid4_rx_priv = NULL);
-	return ccid4_rx_priv;
+    struct ccid4_hc_rx_sock *hcrx = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
+    BUG_ON(hcrx = NULL);
+    return hcrx;
 }

At least this is obvious. Tabulators should be used for indentation here. These inline functions finally ended up in lib/tfrc_ccids.h.

Łeandro Sales

author

2007-11-01 21:44 UTC #2 lore raw

In reply to Leandro

2007/11/1, Tommi Saviranta <[email protected]>:

Quoted 37 lines · Leandro

Quoted 3 lines
[CCID-4] Adapt CCID-4 according to the latest changes to CCID-3
Signed-off-by Leandro Melo de Sales <[email protected]>
Another hit from whitespace police. This isn't really about Leandro's code, but generally about whitespace usage in Linux kernel. Obviously people prefer aligning things:
Quoted 3 lines
      .ccid_owner                = THIS_MODULE,
      .ccid_hc_tx_obj_size       = sizeof(struct ccid4_hc_tx_sock),
      .ccid_hc_tx_init           = ccid4_hc_tx_init,
Sometimes tabulators are used to fill up most of the gap, followed by spaces (like in this case). Sometimes only spaces are used, even for gaps longer seven characters. Is there a general consensus about this? Do the spaces/tabs just get mixed as the code evolves?
Quoted 19 lines
Index: leandro.new/net/dccp/ccids/ccid4.h =================================> > --- leandro.new.orig/net/dccp/ccids/ccid4.h +++ leandro.new/net/dccp/ccids/ccid4.h
(quoted diff) +3 -4
@@ -128,10 +128,9 @@ struct ccid4_hc_tx_sock {
 
 static inline struct ccid4_hc_tx_sock *ccid4_hc_tx_sk(const struct sock *sk)
 {
-     void *ccid4_tx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
-
-     BUG_ON(ccid4_tx_priv = NULL);
-     return ccid4_tx_priv;
+    struct ccid4_hc_tx_sock *hctx = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
+    BUG_ON(hctx = NULL);
+    return hctx;
 }
 
 /* CCID4 receiver states */
@@ -177,10 +176,9 @@ struct ccid4_hc_rx_sock {
 

Quoted 15 lines

Review struct ccid4_hc_rx_sock { · line 182
Quoted 10 lines
(quoted diff) +3 -4
 static inline struct ccid4_hc_rx_sock *ccid4_hc_rx_sk(const struct sock *sk)
 {
-     void *ccid4_rx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
-
-     BUG_ON(ccid4_rx_priv = NULL);
-     return ccid4_rx_priv;
+    struct ccid4_hc_rx_sock *hcrx = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
+    BUG_ON(hcrx = NULL);
+    return hcrx;
 }
At least this is obvious. Tabulators should be used for indentation here. These inline functions finally ended up in lib/tfrc_ccids.h.

I think that we can now concentrate our efforts on the ccid-4 implementation and pay attention for the next coding. As you said, many of the code is from ccid-3, then my suggestion is to focus the job on the ccid-4 implementation and ccid-3/4 code sharing. After we can quickly solve issues related to code style, what do you think?

Leandro

Tommi Saviranta

2007-11-01 22:14 UTC #3 lore raw

Re: [PATCH 9/25] Adapt CCID-4 according to the latest changes to

In reply to Leandro

Quoted 3 lines · Å�eandro Sales

my suggestion is to focus the job on the ccid-4 implementation and ccid-3/4 code sharing. After we can quickly solve issues related to code style, what do you think?

That's fine with me. No matter how it may seem, I'm really not hunting for whitespaces.

Talking of CCID 4 implementation, when would it be safe to start porting changes from old codebase to the new one? I have some Loss Interval and Dropped Packets -related code available for the old repository, but I find it somewhat pointless to submit patches when I know the repository will be written from scratch again, as it was just recently.

Gerrit Renker

2007-11-08 11:27 UTC #4 lore raw

In reply to Leandro

|  static inline struct ccid4_hc_tx_sock *ccid4_hc_tx_sk(const struct sock *sk)
|  {
| -	void *ccid4_tx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
| -
| -	BUG_ON(ccid4_tx_priv = NULL);
| -	return ccid4_tx_priv;
| +    struct ccid4_hc_tx_sock *hctx = ccid_priv(dccp_sk(sk)->dccps_hc_tx_ccid);
| +    BUG_ON(hctx = NULL);
| +    return hctx;

| @@ -177,10 +176,9 @@ struct ccid4_hc_rx_sock {
|  
|  static inline struct ccid4_hc_rx_sock *ccid4_hc_rx_sk(const struct sock *sk)
|  {
| -	void *ccid4_rx_priv = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
| -
| -	BUG_ON(ccid4_rx_priv = NULL);
| -	return ccid4_rx_priv;
| +    struct ccid4_hc_rx_sock *hcrx = ccid_priv(dccp_sk(sk)->dccps_hc_rx_ccid);
| +    BUG_ON(hcrx = NULL);
| +    return hcrx;

Two instances of re-indenting - also fixed.