[PATCH 0/6] net: ceph: messenger: code cleanup

[PATCH 0/6] net: ceph: messenger: code cleanup

[Ioana Ciornei]

This patchset makes some small code cleanup on the messenger code from net/ceph.
Most of the patches are a result of running checkpatch.pl on
net/ceph/messenger.c


Ioana Ciornei (6):
  net: ceph: messenger: remove unnecessary blank spaces
  net: ceph: messenger: fix coding style comparison to NULL
  net: ceph: messenger: add blank line after declarations
  net: ceph: messenger: properly align function parameters
  net: ceph: messenger: change printk to pr_warn
  net: ceph: messenger: add braces to all arms of conditional statement

 net/ceph/messenger.c | 207 ++++++++++++++++++++++++++-------------------------
 1 file changed, 107 insertions(+), 100 deletions(-)

-- 
2.6.4

[PATCH 1/6] net: ceph: messenger: remove unnecessary blank spaces

[Ioana Ciornei]

This patch removes unnecessary blank spaces for the following reasons:
    - sizeof() should be treated as a function so there should not be a
      space between the function name and the open bracket
    - there should not be a space after a cast
    - multiple blank lines are prohibited
    - there is no need for a blank line before a closed bracket '}'

Along these changes changing a call to min_t() was done.

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
 net/ceph/messenger.c | 73 +++++++++++++++++++++-------------------------------
 1 file changed, 30 insertions(+), 43 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 9cfedf5..424d76b 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -195,8 +195,8 @@ const char *ceph_pr_addr(const struct sockaddr_storage *ss)
 {
 	int i;
 	char *s;
-	struct sockaddr_in *in4 = (struct sockaddr_in *) ss;
-	struct sockaddr_in6 *in6 = (struct sockaddr_in6 *) ss;
+	struct sockaddr_in *in4 = (struct sockaddr_in *)ss;
+	struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)ss;
 
 	i = atomic_inc_return(&addr_str_seq) & ADDR_STR_COUNT_MASK;
 	s = addr_str[i];
@@ -236,7 +236,7 @@ static int ceph_msgr_slab_init(void)
 {
 	BUG_ON(ceph_msg_cache);
 	ceph_msg_cache = kmem_cache_create("ceph_msg",
-					sizeof (struct ceph_msg),
+					sizeof(struct ceph_msg),
 					__alignof__(struct ceph_msg), 0, NULL);
 
 	if (!ceph_msg_cache)
@@ -244,7 +244,7 @@ static int ceph_msgr_slab_init(void)
 
 	BUG_ON(ceph_msg_data_cache);
 	ceph_msg_data_cache = kmem_cache_create("ceph_msg_data",
-					sizeof (struct ceph_msg_data),
+					sizeof(struct ceph_msg_data),
 					__alignof__(struct ceph_msg_data),
 					0, NULL);
 	if (ceph_msg_data_cache)
@@ -463,7 +463,6 @@ static void set_sock_callbacks(struct socket *sock,
 	sk->sk_state_change = ceph_sock_state_change;
 }
 
-
 /*
  * socket helpers
  */
@@ -709,7 +708,7 @@ void ceph_con_open(struct ceph_connection *con,
 	WARN_ON(con->state != CON_STATE_CLOSED);
 	con->state = CON_STATE_PREOPEN;
 
-	con->peer_name.type = (__u8) entity_type;
+	con->peer_name.type = (__u8)entity_type;
 	con->peer_name.num = cpu_to_le64(entity_num);
 
 	memcpy(&con->peer_addr, addr, sizeof(*addr));
@@ -751,7 +750,6 @@ void ceph_con_init(struct ceph_connection *con, void *private,
 }
 EXPORT_SYMBOL(ceph_con_init);
 
-
 /*
  * We maintain a global counter to order connection attempts.  Get
  * a unique seq greater than @gt.
@@ -852,12 +850,12 @@ static struct page *ceph_msg_data_bio_next(struct ceph_msg_data_cursor *cursor,
 
 	bio_vec = bio_iter_iovec(bio, cursor->bvec_iter);
 
-	*page_offset = (size_t) bio_vec.bv_offset;
+	*page_offset = (size_t)bio_vec.bv_offset;
 	BUG_ON(*page_offset >= PAGE_SIZE);
 	if (cursor->last_piece) /* pagelist offset is always 0 */
 		*length = cursor->resid;
 	else
-		*length = (size_t) bio_vec.bv_len;
+		*length = (size_t)bio_vec.bv_len;
 	BUG_ON(*length > cursor->resid);
 	BUG_ON(*page_offset + *length > PAGE_SIZE);
 
@@ -1252,10 +1250,10 @@ static void prepare_write_message(struct ceph_connection *con)
 	 * TCP packet that's a good thing. */
 	if (con->in_seq > con->in_seq_acked) {
 		con->in_seq_acked = con->in_seq;
-		con_out_kvec_add(con, sizeof (tag_ack), &tag_ack);
+		con_out_kvec_add(con, sizeof(tag_ack), &tag_ack);
 		con->out_temp_ack = cpu_to_le64(con->in_seq_acked);
-		con_out_kvec_add(con, sizeof (con->out_temp_ack),
-			&con->out_temp_ack);
+		con_out_kvec_add(con, sizeof(con->out_temp_ack),
+				 &con->out_temp_ack);
 	}
 
 	BUG_ON(list_empty(&con->out_queue));
@@ -1284,7 +1282,7 @@ static void prepare_write_message(struct ceph_connection *con)
 	BUG_ON(le32_to_cpu(m->hdr.front_len) != m->front.iov_len);
 
 	/* tag + hdr + front + middle */
-	con_out_kvec_add(con, sizeof (tag_msg), &tag_msg);
+	con_out_kvec_add(con, sizeof(tag_msg), &tag_msg);
 	con_out_kvec_add(con, sizeof(con->out_hdr), &con->out_hdr);
 	con_out_kvec_add(con, m->front.iov_len, m->front.iov_base);
 
@@ -1335,11 +1333,11 @@ static void prepare_write_ack(struct ceph_connection *con)
 
 	con_out_kvec_reset(con);
 
-	con_out_kvec_add(con, sizeof (tag_ack), &tag_ack);
+	con_out_kvec_add(con, sizeof(tag_ack), &tag_ack);
 
 	con->out_temp_ack = cpu_to_le64(con->in_seq_acked);
-	con_out_kvec_add(con, sizeof (con->out_temp_ack),
-				&con->out_temp_ack);
+	con_out_kvec_add(con, sizeof(con->out_temp_ack),
+			 &con->out_temp_ack);
 
 	con->out_more = 1;  /* more will follow.. eventually.. */
 	con_flag_set(con, CON_FLAG_WRITE_PENDING);
@@ -1357,7 +1355,7 @@ static void prepare_write_seq(struct ceph_connection *con)
 	con_out_kvec_reset(con);
 
 	con->out_temp_ack = cpu_to_le64(con->in_seq_acked);
-	con_out_kvec_add(con, sizeof (con->out_temp_ack),
+	con_out_kvec_add(con, sizeof(con->out_temp_ack),
 			 &con->out_temp_ack);
 
 	con_flag_set(con, CON_FLAG_WRITE_PENDING);
@@ -1419,8 +1417,8 @@ static struct ceph_auth_handshake *get_connect_authorizer(struct ceph_connection
 static void prepare_write_banner(struct ceph_connection *con)
 {
 	con_out_kvec_add(con, strlen(CEPH_BANNER), CEPH_BANNER);
-	con_out_kvec_add(con, sizeof (con->msgr->my_enc_addr),
-					&con->msgr->my_enc_addr);
+	con_out_kvec_add(con, sizeof(con->msgr->my_enc_addr),
+			 &con->msgr->my_enc_addr);
 
 	con->out_more = 0;
 	con_flag_set(con, CON_FLAG_WRITE_PENDING);
@@ -1467,8 +1465,8 @@ static int prepare_write_connect(struct ceph_connection *con)
 	con->out_connect.authorizer_len = auth ?
 		cpu_to_le32(auth->authorizer_buf_len) : 0;
 
-	con_out_kvec_add(con, sizeof (con->out_connect),
-					&con->out_connect);
+	con_out_kvec_add(con, sizeof(con->out_connect),
+			 &con->out_connect);
 	if (auth && auth->authorizer_buf_len)
 		con_out_kvec_add(con, auth->authorizer_buf_len,
 					auth->authorizer_buf);
@@ -1607,7 +1605,7 @@ static int write_partial_skip(struct ceph_connection *con)
 
 	dout("%s %p %d left\n", __func__, con, con->out_skip);
 	while (con->out_skip > 0) {
-		size_t size = min(con->out_skip, (int) PAGE_CACHE_SIZE);
+		size_t size = min_t(int, con->out_skip, (int)PAGE_CACHE_SIZE);
 
 		ret = ceph_tcp_sendpage(con->sock, zero_page, 0, size, true);
 		if (ret <= 0)
@@ -1672,7 +1670,6 @@ static int prepare_read_message(struct ceph_connection *con)
 	return 0;
 }
 
-
 static int read_partial(struct ceph_connection *con,
 			int end, int size, void *object)
 {
@@ -1687,7 +1684,6 @@ static int read_partial(struct ceph_connection *con,
 	return 1;
 }
 
-
 /*
  * Read all or part of the connect-side handshake on a new connection
  */
@@ -1706,13 +1702,13 @@ static int read_partial_banner(struct ceph_connection *con)
 	if (ret <= 0)
 		goto out;
 
-	size = sizeof (con->actual_peer_addr);
+	size = sizeof(con->actual_peer_addr);
 	end += size;
 	ret = read_partial(con, end, size, &con->actual_peer_addr);
 	if (ret <= 0)
 		goto out;
 
-	size = sizeof (con->peer_addr_for_me);
+	size = sizeof(con->peer_addr_for_me);
 	end += size;
 	ret = read_partial(con, end, size, &con->peer_addr_for_me);
 	if (ret <= 0)
@@ -1730,7 +1726,7 @@ static int read_partial_connect(struct ceph_connection *con)
 
 	dout("read_partial_connect %p at %d\n", con, con->in_base_pos);
 
-	size = sizeof (con->in_reply);
+	size = sizeof(con->in_reply);
 	end = size;
 	ret = read_partial(con, end, size, &con->in_reply);
 	if (ret <= 0)
@@ -1748,7 +1744,6 @@ static int read_partial_connect(struct ceph_connection *con)
 	     le32_to_cpu(con->in_reply.global_seq));
 out:
 	return ret;
-
 }
 
 /*
@@ -1809,8 +1804,8 @@ static void addr_set_port(struct sockaddr_storage *ss, int p)
 static int ceph_pton(const char *str, size_t len, struct sockaddr_storage *ss,
 		char delim, const char **ipend)
 {
-	struct sockaddr_in *in4 = (struct sockaddr_in *) ss;
-	struct sockaddr_in6 *in6 = (struct sockaddr_in6 *) ss;
+	struct sockaddr_in *in4 = (struct sockaddr_in *)ss;
+	struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)ss;
 
 	memset(ss, 0, sizeof(*ss));
 
@@ -2188,13 +2183,12 @@ static int process_connect(struct ceph_connection *con)
 	return 0;
 }
 
-
 /*
  * read (part of) an ack
  */
 static int read_partial_ack(struct ceph_connection *con)
 {
-	int size = sizeof (con->in_temp_ack);
+	int size = sizeof(con->in_temp_ack);
 	int end = size;
 
 	return read_partial(con, end, size, &con->in_temp_ack);
@@ -2223,7 +2217,6 @@ static void process_ack(struct ceph_connection *con)
 	prepare_read_tag(con);
 }
 
-
 static int read_partial_message_section(struct ceph_connection *con,
 					struct kvec *section,
 					unsigned int sec_len, u32 *crc)
@@ -2276,7 +2269,7 @@ static int read_partial_msg_data(struct ceph_connection *con)
 
 		if (do_datacrc)
 			crc = ceph_crc32c_page(crc, page, page_offset, ret);
-		(void) ceph_msg_data_advance(cursor, (size_t)ret);
+		(void)ceph_msg_data_advance(cursor, (size_t)ret);
 	}
 	if (do_datacrc)
 		con->in_data_crc = crc;
@@ -2304,7 +2297,7 @@ static int read_partial_message(struct ceph_connection *con)
 	dout("read_partial_message con %p msg %p\n", con, m);
 
 	/* header */
-	size = sizeof (con->in_hdr);
+	size = sizeof(con->in_hdr);
 	end = size;
 	ret = read_partial(con, end, size, &con->in_hdr);
 	if (ret <= 0)
@@ -2585,8 +2578,6 @@ out:
 	return ret;
 }
 
-
-
 /*
  * Read what we can from the socket.
  */
@@ -2651,7 +2642,7 @@ more:
 		 * FIXME: there must be a better way to do this!
 		 */
 		static char buf[SKIP_BUF_SIZE];
-		int skip = min((int) sizeof (buf), -con->in_base_pos);
+		int skip = min((int)sizeof(buf), -con->in_base_pos);
 
 		dout("skipping %d / %d bytes\n", skip, -con->in_base_pos);
 		ret = ceph_tcp_recvmsg(con->sock, buf, skip);
@@ -2740,7 +2731,6 @@ bad_tag:
 	goto out;
 }
 
-
 /*
  * Atomically queue work on a connection after the specified delay.
  * Bump @con reference to avoid races with connection teardown.
@@ -2765,7 +2755,7 @@ static int queue_con_delay(struct ceph_connection *con, unsigned long delay)
 
 static void queue_con(struct ceph_connection *con)
 {
-	(void) queue_con_delay(con, 0);
+	(void)queue_con_delay(con, 0);
 }
 
 static void cancel_con(struct ceph_connection *con)
@@ -2963,8 +2953,6 @@ static void con_fault(struct ceph_connection *con)
 	}
 }
 
-
-
 /*
  * initialize a new messenger instance
  */
@@ -3388,7 +3376,6 @@ static int ceph_con_in_msg_alloc(struct ceph_connection *con, int *skip)
 	return ret;
 }
 
-
 /*
  * Free a generically kmalloc'd message.
  */
-- 
2.6.4

[PATCH 2/6] net: ceph: messenger: fix coding style comparison to NULL

[Ioana Ciornei]

This patch fixes the comparison to NULL format in order to follow linux
kernel coding style.

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
 net/ceph/messenger.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 424d76b..7a8bc51 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -3249,7 +3249,7 @@ struct ceph_msg *ceph_msg_new(int type, int front_len, gfp_t flags,
 	struct ceph_msg *m;
 
 	m = kmem_cache_zalloc(ceph_msg_cache, flags);
-	if (m == NULL)
+	if (!m)
 		goto out;
 
 	m->hdr.type = cpu_to_le16(type);
@@ -3263,7 +3263,7 @@ struct ceph_msg *ceph_msg_new(int type, int front_len, gfp_t flags,
 	/* front */
 	if (front_len) {
 		m->front.iov_base = ceph_kvmalloc(front_len, flags);
-		if (m->front.iov_base == NULL) {
+		if (!m->front.iov_base) {
 			dout("ceph_msg_new can't allocate %d bytes\n",
 			     front_len);
 			goto out2;
-- 
2.6.4

[PATCH 3/6] net: ceph: messenger: add blank line after declarations

[Ioana Ciornei]

This patch adds a blank line after declarations in order to follow the
kernel coding style. Also, it adds spaces around a binary operator.

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
 net/ceph/messenger.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 7a8bc51..6a0962d 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -389,6 +389,7 @@ static void con_sock_state_closed(struct ceph_connection *con)
 static void ceph_sock_data_ready(struct sock *sk)
 {
 	struct ceph_connection *con = sk->sk_user_data;
+
 	if (atomic_read(&con->msgr->stopping)) {
 		return;
 	}
@@ -457,6 +458,7 @@ static void set_sock_callbacks(struct socket *sock,
 			       struct ceph_connection *con)
 {
 	struct sock *sk = sock->sk;
+
 	sk->sk_user_data = con;
 	sk->sk_data_ready = ceph_sock_data_ready;
 	sk->sk_write_space = ceph_sock_write_space;
@@ -636,6 +638,7 @@ static void ceph_msg_remove(struct ceph_msg *msg)
 
 	ceph_msg_put(msg);
 }
+
 static void ceph_msg_remove_list(struct list_head *head)
 {
 	while (!list_empty(head)) {
@@ -1384,7 +1387,6 @@ static void prepare_write_keepalive(struct ceph_connection *con)
 /*
  * Connection negotiation.
  */
-
 static struct ceph_auth_handshake *get_connect_authorizer(struct ceph_connection *con,
 						int *auth_proto)
 {
@@ -1543,6 +1545,7 @@ static int write_partial_message_data(struct ceph_connection *con)
 {
 	struct ceph_msg *msg = con->out_msg;
 	struct ceph_msg_data_cursor *cursor = &msg->cursor;
+
 	bool do_datacrc = !ceph_test_opt(from_msgr(con->msgr), NOCRC);
 	u32 crc;
 
@@ -1677,6 +1680,7 @@ static int read_partial(struct ceph_connection *con,
 		int left = end - con->in_base_pos;
 		int have = size - left;
 		int ret = ceph_tcp_recvmsg(con->sock, object + have, left);
+
 		if (ret <= 0)
 			return ret;
 		con->in_base_pos += ret;
@@ -1904,7 +1908,7 @@ int ceph_parse_ips(const char *c, const char *end,
 	int i, ret = -EINVAL;
 	const char *p = c;
 
-	dout("parse_ips on '%.*s'\n", (int)(end-c), c);
+	dout("parse_ips on '%.*s'\n", (int)(end - c), c);
 	for (i = 0; i < max_count; i++) {
 		const char *ipend;
 		struct sockaddr_storage *ss = &addr[i].in_addr;
@@ -2479,6 +2483,7 @@ static int read_keepalive_ack(struct ceph_connection *con)
 	struct ceph_timespec ceph_ts;
 	size_t size = sizeof(ceph_ts);
 	int ret = read_partial(con, size, size, &ceph_ts);
+
 	if (ret <= 0)
 		return ret;
 	ceph_decode_timespec(&con->last_keepalive_ack, &ceph_ts);
@@ -3151,6 +3156,7 @@ bool ceph_con_keepalive_expired(struct ceph_connection *con,
 	    (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) {
 		struct timespec now = CURRENT_TIME;
 		struct timespec ts;
+
 		jiffies_to_timespec(interval, &ts);
 		ts = timespec_add(con->last_keepalive_ack, ts);
 		return timespec_compare(&now, &ts) >= 0;
-- 
2.6.4

[PATCH 4/6] net: ceph: messenger: properly align function parameters

[Ioana Ciornei]

This patch properly aligns function parameters and function call
arguments to match the open bracket '(' in order to follow the linux
kernel coding style.

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
 net/ceph/messenger.c | 97 ++++++++++++++++++++++++++++------------------------
 1 file changed, 52 insertions(+), 45 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 6a0962d..6b15876 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -136,7 +136,7 @@ static bool con_flag_test(struct ceph_connection *con, unsigned long con_flag)
 }
 
 static bool con_flag_test_and_clear(struct ceph_connection *con,
-					unsigned long con_flag)
+				    unsigned long con_flag)
 {
 	BUG_ON(!con_flag_valid(con_flag));
 
@@ -144,7 +144,7 @@ static bool con_flag_test_and_clear(struct ceph_connection *con,
 }
 
 static bool con_flag_test_and_set(struct ceph_connection *con,
-					unsigned long con_flag)
+				  unsigned long con_flag)
 {
 	BUG_ON(!con_flag_valid(con_flag));
 
@@ -236,8 +236,10 @@ static int ceph_msgr_slab_init(void)
 {
 	BUG_ON(ceph_msg_cache);
 	ceph_msg_cache = kmem_cache_create("ceph_msg",
-					sizeof(struct ceph_msg),
-					__alignof__(struct ceph_msg), 0, NULL);
+					   sizeof(struct ceph_msg),
+					   __alignof__(struct ceph_msg),
+					   0,
+					   NULL);
 
 	if (!ceph_msg_cache)
 		return -ENOMEM;
@@ -360,8 +362,8 @@ static void con_sock_state_closing(struct ceph_connection *con)
 
 	old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSING);
 	if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING &&
-			old_state != CON_SOCK_STATE_CONNECTED &&
-			old_state != CON_SOCK_STATE_CLOSING))
+		    old_state != CON_SOCK_STATE_CONNECTED &&
+		    old_state != CON_SOCK_STATE_CLOSING))
 		printk("%s: unexpected old state %d\n", __func__, old_state);
 	dout("%s con %p sock %d -> %d\n", __func__, con, old_state,
 	     CON_SOCK_STATE_CLOSING);
@@ -534,7 +536,7 @@ static int ceph_tcp_recvmsg(struct socket *sock, void *buf, size_t len)
 }
 
 static int ceph_tcp_recvpage(struct socket *sock, struct page *page,
-		     int page_offset, size_t length)
+			     int page_offset, size_t length)
 {
 	void *kaddr;
 	int ret;
@@ -554,7 +556,7 @@ static int ceph_tcp_recvpage(struct socket *sock, struct page *page,
  * shortly.
  */
 static int ceph_tcp_sendmsg(struct socket *sock, struct kvec *iov,
-		     size_t kvlen, size_t len, int more)
+			    size_t kvlen, size_t len, int more)
 {
 	struct msghdr msg = { .msg_flags = MSG_DONTWAIT | MSG_NOSIGNAL };
 	int r;
@@ -571,7 +573,7 @@ static int ceph_tcp_sendmsg(struct socket *sock, struct kvec *iov,
 }
 
 static int __ceph_tcp_sendpage(struct socket *sock, struct page *page,
-		     int offset, size_t size, bool more)
+			       int offset, size_t size, bool more)
 {
 	int flags = MSG_DONTWAIT | MSG_NOSIGNAL | (more ? MSG_MORE : MSG_EOR);
 	int ret;
@@ -584,7 +586,7 @@ static int __ceph_tcp_sendpage(struct socket *sock, struct page *page,
 }
 
 static int ceph_tcp_sendpage(struct socket *sock, struct page *page,
-		     int offset, size_t size, bool more)
+			     int offset, size_t size, bool more)
 {
 	int ret;
 	struct kvec iov;
@@ -733,8 +735,8 @@ bool ceph_con_opened(struct ceph_connection *con)
  * initialize a new connection.
  */
 void ceph_con_init(struct ceph_connection *con, void *private,
-	const struct ceph_connection_operations *ops,
-	struct ceph_messenger *msgr)
+		   const struct ceph_connection_operations *ops,
+		   struct ceph_messenger *msgr)
 {
 	dout("con_init %p\n", con);
 	memset(con, 0, sizeof(*con));
@@ -779,7 +781,7 @@ static void con_out_kvec_reset(struct ceph_connection *con)
 }
 
 static void con_out_kvec_add(struct ceph_connection *con,
-				size_t size, void *data)
+			     size_t size, void *data)
 {
 	int index = con->out_kvec_left;
 
@@ -821,7 +823,7 @@ static int con_out_kvec_skip(struct ceph_connection *con)
  * bio in the list.
  */
 static void ceph_msg_data_bio_cursor_init(struct ceph_msg_data_cursor *cursor,
-					size_t length)
+					  size_t length)
 {
 	struct ceph_msg_data *data = cursor->data;
 	struct bio *bio;
@@ -839,8 +841,8 @@ static void ceph_msg_data_bio_cursor_init(struct ceph_msg_data_cursor *cursor,
 }
 
 static struct page *ceph_msg_data_bio_next(struct ceph_msg_data_cursor *cursor,
-						size_t *page_offset,
-						size_t *length)
+					   size_t *page_offset,
+					   size_t *length)
 {
 	struct ceph_msg_data *data = cursor->data;
 	struct bio *bio;
@@ -866,7 +868,7 @@ static struct page *ceph_msg_data_bio_next(struct ceph_msg_data_cursor *cursor,
 }
 
 static bool ceph_msg_data_bio_advance(struct ceph_msg_data_cursor *cursor,
-					size_t bytes)
+				      size_t bytes)
 {
 	struct bio *bio;
 	struct bio_vec bio_vec;
@@ -917,7 +919,7 @@ static bool ceph_msg_data_bio_advance(struct ceph_msg_data_cursor *cursor,
  * that has not already been fully consumed.
  */
 static void ceph_msg_data_pages_cursor_init(struct ceph_msg_data_cursor *cursor,
-					size_t length)
+					    size_t length)
 {
 	struct ceph_msg_data *data = cursor->data;
 	int page_count;
@@ -939,7 +941,7 @@ static void ceph_msg_data_pages_cursor_init(struct ceph_msg_data_cursor *cursor,
 
 static struct page *
 ceph_msg_data_pages_next(struct ceph_msg_data_cursor *cursor,
-					size_t *page_offset, size_t *length)
+			 size_t *page_offset, size_t *length)
 {
 	struct ceph_msg_data *data = cursor->data;
 
@@ -958,7 +960,7 @@ ceph_msg_data_pages_next(struct ceph_msg_data_cursor *cursor,
 }
 
 static bool ceph_msg_data_pages_advance(struct ceph_msg_data_cursor *cursor,
-						size_t bytes)
+					size_t bytes)
 {
 	BUG_ON(cursor->data->type != CEPH_MSG_DATA_PAGES);
 
@@ -989,7 +991,7 @@ static bool ceph_msg_data_pages_advance(struct ceph_msg_data_cursor *cursor,
  */
 static void
 ceph_msg_data_pagelist_cursor_init(struct ceph_msg_data_cursor *cursor,
-					size_t length)
+				   size_t length)
 {
 	struct ceph_msg_data *data = cursor->data;
 	struct ceph_pagelist *pagelist;
@@ -1014,7 +1016,7 @@ ceph_msg_data_pagelist_cursor_init(struct ceph_msg_data_cursor *cursor,
 
 static struct page *
 ceph_msg_data_pagelist_next(struct ceph_msg_data_cursor *cursor,
-				size_t *page_offset, size_t *length)
+			    size_t *page_offset, size_t *length)
 {
 	struct ceph_msg_data *data = cursor->data;
 	struct ceph_pagelist *pagelist;
@@ -1038,7 +1040,7 @@ ceph_msg_data_pagelist_next(struct ceph_msg_data_cursor *cursor,
 }
 
 static bool ceph_msg_data_pagelist_advance(struct ceph_msg_data_cursor *cursor,
-						size_t bytes)
+					   size_t bytes)
 {
 	struct ceph_msg_data *data = cursor->data;
 	struct ceph_pagelist *pagelist;
@@ -1126,8 +1128,8 @@ static void ceph_msg_data_cursor_init(struct ceph_msg *msg, size_t length)
  * Indicate whether this is the last piece in this data item.
  */
 static struct page *ceph_msg_data_next(struct ceph_msg_data_cursor *cursor,
-					size_t *page_offset, size_t *length,
-					bool *last_piece)
+				       size_t *page_offset, size_t *length,
+				       bool *last_piece)
 {
 	struct page *page;
 
@@ -1162,7 +1164,7 @@ static struct page *ceph_msg_data_next(struct ceph_msg_data_cursor *cursor,
  * of the data item.
  */
 static bool ceph_msg_data_advance(struct ceph_msg_data_cursor *cursor,
-				size_t bytes)
+				  size_t bytes)
 {
 	bool new_piece;
 
@@ -1291,7 +1293,7 @@ static void prepare_write_message(struct ceph_connection *con)
 
 	if (m->middle)
 		con_out_kvec_add(con, m->middle->vec.iov_len,
-			m->middle->vec.iov_base);
+				 m->middle->vec.iov_base);
 
 	/* fill in hdr crc and finalize hdr */
 	crc = crc32c(0, &m->hdr, offsetof(struct ceph_msg_header, crc));
@@ -1302,8 +1304,9 @@ static void prepare_write_message(struct ceph_connection *con)
 	crc = crc32c(0, m->front.iov_base, m->front.iov_len);
 	con->out_msg->footer.front_crc = cpu_to_le32(crc);
 	if (m->middle) {
-		crc = crc32c(0, m->middle->vec.iov_base,
-				m->middle->vec.iov_len);
+		crc = crc32c(0,
+			     m->middle->vec.iov_base,
+			     m->middle->vec.iov_len);
 		con->out_msg->footer.middle_crc = cpu_to_le32(crc);
 	} else
 		con->out_msg->footer.middle_crc = 0;
@@ -1387,8 +1390,9 @@ static void prepare_write_keepalive(struct ceph_connection *con)
 /*
  * Connection negotiation.
  */
-static struct ceph_auth_handshake *get_connect_authorizer(struct ceph_connection *con,
-						int *auth_proto)
+static struct
+ceph_auth_handshake *get_connect_authorizer(struct ceph_connection *con,
+					    int *auth_proto)
 {
 	struct ceph_auth_handshake *auth;
 
@@ -1471,7 +1475,7 @@ static int prepare_write_connect(struct ceph_connection *con)
 			 &con->out_connect);
 	if (auth && auth->authorizer_buf_len)
 		con_out_kvec_add(con, auth->authorizer_buf_len,
-					auth->authorizer_buf);
+				 auth->authorizer_buf);
 
 	con->out_more = 0;
 	con_flag_set(con, CON_FLAG_WRITE_PENDING);
@@ -1522,8 +1526,8 @@ out:
 }
 
 static u32 ceph_crc32c_page(u32 crc, struct page *page,
-				unsigned int page_offset,
-				unsigned int length)
+			    unsigned int page_offset,
+			    unsigned int length)
 {
 	char *kaddr;
 
@@ -1806,7 +1810,7 @@ static void addr_set_port(struct sockaddr_storage *ss, int p)
  * Unlike other *_pton function semantics, zero indicates success.
  */
 static int ceph_pton(const char *str, size_t len, struct sockaddr_storage *ss,
-		char delim, const char **ipend)
+		     char delim, const char **ipend)
 {
 	struct sockaddr_in *in4 = (struct sockaddr_in *)ss;
 	struct sockaddr_in6 *in6 = (struct sockaddr_in6 *)ss;
@@ -1831,7 +1835,8 @@ static int ceph_pton(const char *str, size_t len, struct sockaddr_storage *ss,
  */
 #ifdef CONFIG_CEPH_LIB_USE_DNS_RESOLVER
 static int ceph_dns_resolve_name(const char *name, size_t namelen,
-		struct sockaddr_storage *ss, char delim, const char **ipend)
+				 struct sockaddr_storage *ss,
+				 char delim, const char **ipend)
 {
 	const char *end, *delim_p;
 	char *colon_p, *ip_addr = NULL;
@@ -1869,13 +1874,14 @@ static int ceph_dns_resolve_name(const char *name, size_t namelen,
 	*ipend = end;
 
 	pr_info("resolve '%.*s' (ret=%d): %s\n", (int)(end - name), name,
-			ret, ret ? "failed" : ceph_pr_addr(ss));
+		ret, ret ? "failed" : ceph_pr_addr(ss));
 
 	return ret;
 }
 #else
 static inline int ceph_dns_resolve_name(const char *name, size_t namelen,
-		struct sockaddr_storage *ss, char delim, const char **ipend)
+					struct sockaddr_storage *ss,
+					char delim, const char **ipend)
 {
 	return -EINVAL;
 }
@@ -1886,7 +1892,8 @@ static inline int ceph_dns_resolve_name(const char *name, size_t namelen,
  * then try to extract a hostname to resolve using userspace DNS upcall.
  */
 static int ceph_parse_server_name(const char *name, size_t namelen,
-			struct sockaddr_storage *ss, char delim, const char **ipend)
+				  struct sockaddr_storage *ss,
+				  char delim, const char **ipend)
 {
 	int ret;
 
@@ -2810,7 +2817,7 @@ static bool con_backoff(struct ceph_connection *con)
 	ret = queue_con_delay(con, round_jiffies_relative(con->delay));
 	if (ret) {
 		dout("%s: con %p FAILED to back off %lu\n", __func__,
-			con, con->delay);
+		     con, con->delay);
 		BUG_ON(ret == -ENOENT);
 		con_flag_set(con, CON_FLAG_BACKOFF);
 	}
@@ -2919,8 +2926,8 @@ static void con_fault(struct ceph_connection *con)
 	con->error_msg = NULL;
 
 	WARN_ON(con->state != CON_STATE_CONNECTING &&
-	       con->state != CON_STATE_NEGOTIATING &&
-	       con->state != CON_STATE_OPEN);
+		con->state != CON_STATE_NEGOTIATING &&
+		con->state != CON_STATE_OPEN);
 
 	con_close_socket(con);
 
@@ -3150,7 +3157,7 @@ void ceph_con_keepalive(struct ceph_connection *con)
 EXPORT_SYMBOL(ceph_con_keepalive);
 
 bool ceph_con_keepalive_expired(struct ceph_connection *con,
-			       unsigned long interval)
+				unsigned long interval)
 {
 	if (interval > 0 &&
 	    (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2)) {
@@ -3191,7 +3198,7 @@ static void ceph_msg_data_destroy(struct ceph_msg_data *data)
 }
 
 void ceph_msg_data_add_pages(struct ceph_msg *msg, struct page **pages,
-		size_t length, size_t alignment)
+			     size_t length, size_t alignment)
 {
 	struct ceph_msg_data *data;
 
@@ -3228,7 +3235,7 @@ EXPORT_SYMBOL(ceph_msg_data_add_pagelist);
 
 #ifdef	CONFIG_BLOCK
 void ceph_msg_data_add_bio(struct ceph_msg *msg, struct bio *bio,
-		size_t length)
+			   size_t length)
 {
 	struct ceph_msg_data *data;
 
-- 
2.6.4

[PATCH 5/6] net: ceph: messenger: change printk to pr_warn

[Ioana Ciornei]

This patch changes the printk call into the preferred function
pr_warn(). Since the intention is to just warn the user it is the best
level of facility.

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
 net/ceph/messenger.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 6b15876..8f1582e 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -329,7 +329,8 @@ static void con_sock_state_init(struct ceph_connection *con)
 
 	old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CLOSED);
 	if (WARN_ON(old_state != CON_SOCK_STATE_NEW))
-		printk("%s: unexpected old state %d\n", __func__, old_state);
+		pr_warn("%s: unexpected old state %d\n", __func__,
+			old_state);
 	dout("%s con %p sock %d -> %d\n", __func__, con, old_state,
 	     CON_SOCK_STATE_CLOSED);
 }
@@ -340,7 +341,8 @@ static void con_sock_state_connecting(struct ceph_connection *con)
 
 	old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CONNECTING);
 	if (WARN_ON(old_state != CON_SOCK_STATE_CLOSED))
-		printk("%s: unexpected old state %d\n", __func__, old_state);
+		pr_warn("%s: unexpected old state %d\n", __func__,
+			old_state);
 	dout("%s con %p sock %d -> %d\n", __func__, con, old_state,
 	     CON_SOCK_STATE_CONNECTING);
 }
@@ -351,7 +353,8 @@ static void con_sock_state_connected(struct ceph_connection *con)
 
 	old_state = atomic_xchg(&con->sock_state, CON_SOCK_STATE_CONNECTED);
 	if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING))
-		printk("%s: unexpected old state %d\n", __func__, old_state);
+		pr_warn("%s: unexpected old state %d\n", __func__,
+			old_state);
 	dout("%s con %p sock %d -> %d\n", __func__, con, old_state,
 	     CON_SOCK_STATE_CONNECTED);
 }
@@ -364,7 +367,8 @@ static void con_sock_state_closing(struct ceph_connection *con)
 	if (WARN_ON(old_state != CON_SOCK_STATE_CONNECTING &&
 		    old_state != CON_SOCK_STATE_CONNECTED &&
 		    old_state != CON_SOCK_STATE_CLOSING))
-		printk("%s: unexpected old state %d\n", __func__, old_state);
+		pr_warn("%s: unexpected old state %d\n", __func__,
+			old_state);
 	dout("%s con %p sock %d -> %d\n", __func__, con, old_state,
 	     CON_SOCK_STATE_CLOSING);
 }
@@ -378,7 +382,8 @@ static void con_sock_state_closed(struct ceph_connection *con)
 		    old_state != CON_SOCK_STATE_CLOSING &&
 		    old_state != CON_SOCK_STATE_CONNECTING &&
 		    old_state != CON_SOCK_STATE_CLOSED))
-		printk("%s: unexpected old state %d\n", __func__, old_state);
+		pr_warn("%s: unexpected old state %d\n", __func__,
+			old_state);
 	dout("%s con %p sock %d -> %d\n", __func__, con, old_state,
 	     CON_SOCK_STATE_CLOSED);
 }
-- 
2.6.4

[PATCH 6/6] net: ceph: messenger: add braces to all arms of conditional statement

[Ioana Ciornei]

This patch adds curly braces to all arms of the conditional statement in
order to follow the linux kernel coding style. Warnings found using
checkpatch.pl.

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---
 net/ceph/messenger.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 8f1582e..eb71add 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -1313,8 +1313,10 @@ static void prepare_write_message(struct ceph_connection *con)
 			     m->middle->vec.iov_base,
 			     m->middle->vec.iov_len);
 		con->out_msg->footer.middle_crc = cpu_to_le32(crc);
-	} else
+	} else {
 		con->out_msg->footer.middle_crc = 0;
+	}
+
 	dout("%s front_crc %u middle_crc %u\n", __func__,
 	     le32_to_cpu(con->out_msg->footer.front_crc),
 	     le32_to_cpu(con->out_msg->footer.middle_crc));
@@ -1854,11 +1856,11 @@ static int ceph_dns_resolve_name(const char *name, size_t namelen,
 	delim_p = memchr(name, delim, namelen);
 	colon_p = memchr(name, ':', namelen);
 
-	if (delim_p && colon_p)
+	if (delim_p && colon_p) {
 		end = delim_p < colon_p ? delim_p : colon_p;
-	else if (!delim_p && colon_p)
+	} else if (!delim_p && colon_p) {
 		end = colon_p;
-	else {
+	} else {
 		end = delim_p;
 		if (!end) /* case: hostname:/ */
 			end = name + namelen;
-- 
2.6.4

Re: [PATCH 0/6] net: ceph: messenger: code cleanup

[Ilya Dryomov]

On Wed, Jan 27, 2016 at 1:43 AM, Ioana Ciornei <ciorneiioana@gmail.com> wrote:
>
> This patchset makes some small code cleanup on the messenger code from net/ceph.
> Most of the patches are a result of running checkpatch.pl on
> net/ceph/messenger.c
>
>
> Ioana Ciornei (6):
>   net: ceph: messenger: remove unnecessary blank spaces
>   net: ceph: messenger: fix coding style comparison to NULL
>   net: ceph: messenger: add blank line after declarations
>   net: ceph: messenger: properly align function parameters
>   net: ceph: messenger: change printk to pr_warn
>   net: ceph: messenger: add braces to all arms of conditional statement
>
>  net/ceph/messenger.c | 207 ++++++++++++++++++++++++++-------------------------
>  1 file changed, 107 insertions(+), 100 deletions(-)

Hi Ioana,

Most of above (especially patch 4/6) just clutters the history (think
"git blame" output) and, more importantly, complicates backporting of
future fixes to older kernels for no benefit at all.

There is a reason checkpatch.pl is called "check" "patch" - it is to be
run on patches, not files, with pretty much the only exception to that
being code in drivers/staging.  In certain cases, it might be OK to use
it on existing files to fix what it classifies as errors, but fixing
warnings or so called checks does more harm than good in most cases.

Thanks,

                Ilya