[1/3] remote: Fix code indentation

Submitted by Radostin Stoyanov on Jan. 29, 2019, 5:37 p.m.

Details

Message ID 20190129173747.30092-1-rstoyanov1@gmail.com
State Accepted
Series "Series without cover letter"
Commit 98b44fb700f8eb944b042317f8a755f6c9c1bba8
Headers show

Commit Message

Radostin Stoyanov Jan. 29, 2019, 5:37 p.m.
This patch does not introduce any functional changes.

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
---
 criu/img-remote.c | 71 +++++++++++++++++++----------------------------
 1 file changed, 28 insertions(+), 43 deletions(-)

Patch hide | download patch | download mbox

diff --git a/criu/img-remote.c b/criu/img-remote.c
index 9f49e92fd..d1acf21d0 100644
--- a/criu/img-remote.c
+++ b/criu/img-remote.c
@@ -357,16 +357,14 @@  static void rop_set_rimg(struct roperation* rop, struct rimage* rimg)
 		if (rop->fd == proxy_to_cache_fd) {
 			rop->curr_sent_buf = rimg->curr_fwd_buf;
 			rop->curr_sent_bytes = rimg->curr_fwd_bytes;
-		}
-		// For local appends, just write at the end.
-		else {
+		} else {
+			// For local appends, just write at the end.
 			rop->curr_sent_buf = list_entry(rimg->buf_head.prev, struct rbuf, l);
 			rop->curr_sent_bytes = rop->curr_sent_buf->nbytes;
 		}
 		// On the receiver size, we just append
 		rop->curr_recv_buf = list_entry(rimg->buf_head.prev, struct rbuf, l);
-	}
-	else {
+	} else {
 		// Writes or reads are simple. Just do it from the beginning.
 		rop->curr_recv_buf = list_entry(rimg->buf_head.next, struct rbuf, l);
 		rop->curr_sent_buf = list_entry(rimg->buf_head.next, struct rbuf, l);
@@ -403,8 +401,7 @@  struct roperation* handle_accept_write(
 			pr_perror("Error preparing remote image");
 			goto err;
 		}
-	}
-	else {
+	} else {
 		list_del(&(rimg->l));
 		if (flags == O_APPEND)
 			clear_remote_image(rimg);
@@ -510,9 +507,8 @@  struct roperation* handle_accept_cache_read(
 		}
 		rop_set_rimg(rop, rimg);
 		return rop;
-	}
-	// The file does not exist.
-	else if (finished_remote) {
+	} else if (finished_remote) {
+		// The file does not exist.
 		pr_info("No image %s:%s.\n", path, snapshot_id);
 		if (write_reply_header(cli_fd, ENOENT) < 0)
 			pr_perror("Error writing reply header for unexisting image");
@@ -628,13 +624,11 @@  void handle_local_accept(int fd)
 	// Write/Append case (only possible in img-proxy).
 	if (flags != O_RDONLY) {
 		rop = handle_accept_proxy_write(cli_fd, snapshot_id, path, flags);
-	}
-	// Read case while restoring (img-cache).
-	else if (restoring) {
+	} else if (restoring) {
+		// Read case while restoring (img-cache).
 		rop = handle_accept_cache_read(cli_fd, snapshot_id, path, flags);
-	}
-	// Read case while dumping (img-proxy).
-	else {
+	} else {
+		// Read case while dumping (img-proxy).
 		rop = handle_accept_proxy_read(cli_fd, snapshot_id, path, flags);
 	}
 
@@ -669,8 +663,8 @@  void finish_proxy_read(struct roperation* rop)
 
 		forwarding = false;
 
-	// If there are images waiting to be forwarded, forward the next.
-	if (!list_empty(&rop_forwarding)) {
+		// If there are images waiting to be forwarded, forward the next.
+		if (!list_empty(&rop_forwarding)) {
 			forward_remote_image(list_entry(rop_forwarding.next, struct roperation, l));
 		}
 	}
@@ -682,9 +676,8 @@  void finish_proxy_write(struct roperation* rop)
 	if (!strncmp(rop->path, DUMP_FINISH, sizeof(DUMP_FINISH))) {
 		// TODO - couldn't we handle the DUMP_FINISH in inside handle_accept_proxy_write?
 		finish_local();
-	}
-	// Normal image received, forward it.
-	else {
+	} else {
+		// Normal image received, forward it.
 		struct roperation *rop_to_forward = new_remote_operation(
 			rop->path, rop->snapshot_id, proxy_to_cache_fd, rop->flags, false);
 
@@ -694,7 +687,7 @@  void finish_proxy_write(struct roperation* rop)
 		rop_set_rimg(rop_to_forward, rop->rimg);
 		if (list_empty(&rop_forwarding)) {
 			forward_remote_image(rop_to_forward);
-	}
+		}
 		list_add_tail(&(rop_to_forward->l), &rop_forwarding);
 	}
 }
@@ -771,14 +764,11 @@  void handle_roperation(struct epoll_event *event, struct roperation *rop)
 		// Cached side (finished receiving forwarded image)
 		if (restoring) {
 			finish_cache_write(rop);
-		}
-		// Proxy side (finished receiving local image)
-		else {
+		} else {
+			// Proxy side (finished receiving local image)
 			finish_proxy_write(rop);
 		}
-	}
-	// If send operation if finished
-	else {
+	} else {
 		// Proxy side (Finished forwarding image or reading it locally).
 		if (!restoring)
 			finish_proxy_read(rop);
@@ -851,19 +841,17 @@  void accept_image_connections() {
 		for (i = 0; i < n_events; i++) {
 			// Accept from local dump/restore?
 			if (events[i].data.ptr == &local_req_fd) {
-				if ( events[i].events & EPOLLHUP ||
+				if (events[i].events & EPOLLHUP ||
 					events[i].events & EPOLLERR) {
 					if (!finished_local)
 						pr_perror("Unable to accept more local image connections");
 					goto end;
 				}
 				handle_local_accept(local_req_fd);
-			}
-			else if (restoring && !forwarding && events[i].data.ptr == &proxy_to_cache_fd) {
+			} else if (restoring && !forwarding && events[i].data.ptr == &proxy_to_cache_fd) {
 				event_set(epoll_fd, EPOLL_CTL_DEL, proxy_to_cache_fd, 0, 0);
 				handle_remote_accept(proxy_to_cache_fd);
-			}
-			else {
+			} else {
 				struct roperation *rop =
 					(struct roperation*)events[i].data.ptr;
 				event_set(epoll_fd, EPOLL_CTL_DEL, rop->fd, 0, 0);
@@ -971,22 +959,19 @@  int64_t send_image_async(struct roperation *op)
 				list_entry(op->curr_sent_buf->l.next, struct rbuf, l);
 			op->curr_sent_bytes = 0;
 			return n;
-		}
-		else if (op->curr_sent_bytes == op->curr_sent_buf->nbytes) {
+		} else if (op->curr_sent_bytes == op->curr_sent_buf->nbytes) {
 			if (close_fd)
 				close(fd);
 			return 0;
 		}
 		return n;
-	}
-	else if (errno == EPIPE || errno == ECONNRESET) {
+	} else if (errno == EPIPE || errno == ECONNRESET) {
 		pr_warn("Connection for %s:%s was closed early than expected\n",
 			rimg->path, rimg->snapshot_id);
 		return 0;
 	} else if (errno == EAGAIN || errno == EWOULDBLOCK) {
 		return errno;
-	}
-	else {
+	} else {
 		pr_perror("Write on %s:%s socket failed",
 			rimg->path, rimg->snapshot_id);
 		return -1;
@@ -1014,9 +999,9 @@  int read_remote_image_connection(char *snapshot_id, char *path)
 				path, snapshot_id);
 		return -1;
 	}
-	if (!error || !strncmp(path, RESTORE_FINISH, sizeof(RESTORE_FINISH)))
+	if (!error || !strncmp(path, RESTORE_FINISH, sizeof(RESTORE_FINISH))) {
 		return sockfd;
-	else if (error == ENOENT) {
+	} else if (error == ENOENT) {
 		pr_info("Image does not exist (%s:%s)\n", path, snapshot_id);
 		close(sockfd);
 		return -ENOENT;
@@ -1184,13 +1169,13 @@  int get_curr_snapshot_id_idx(void)
 		pull_snapshot_ids();
 
 	list_for_each_entry(si, &snapshot_head, l) {
-	if (!strncmp(si->snapshot_id, snapshot_id, PATH_MAX))
+		if (!strncmp(si->snapshot_id, snapshot_id, PATH_MAX))
 			return idx;
 		idx++;
 	}
 
 	pr_err("Error, could not find current snapshot id (%s) fd\n",
-			snapshot_id);
+		snapshot_id);
 	return -1;
 }
 

Comments

Andrei Vagin Feb. 1, 2019, 5:29 p.m.
Accepted, thanks!

On Tue, Jan 29, 2019 at 05:37:45PM +0000, Radostin Stoyanov wrote:
> This patch does not introduce any functional changes.
> 
> Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
> ---
>  criu/img-remote.c | 71 +++++++++++++++++++----------------------------
>  1 file changed, 28 insertions(+), 43 deletions(-)
> 
> diff --git a/criu/img-remote.c b/criu/img-remote.c
> index 9f49e92fd..d1acf21d0 100644
> --- a/criu/img-remote.c
> +++ b/criu/img-remote.c
> @@ -357,16 +357,14 @@ static void rop_set_rimg(struct roperation* rop, struct rimage* rimg)
>  		if (rop->fd == proxy_to_cache_fd) {
>  			rop->curr_sent_buf = rimg->curr_fwd_buf;
>  			rop->curr_sent_bytes = rimg->curr_fwd_bytes;
> -		}
> -		// For local appends, just write at the end.
> -		else {
> +		} else {
> +			// For local appends, just write at the end.
>  			rop->curr_sent_buf = list_entry(rimg->buf_head.prev, struct rbuf, l);
>  			rop->curr_sent_bytes = rop->curr_sent_buf->nbytes;
>  		}
>  		// On the receiver size, we just append
>  		rop->curr_recv_buf = list_entry(rimg->buf_head.prev, struct rbuf, l);
> -	}
> -	else {
> +	} else {
>  		// Writes or reads are simple. Just do it from the beginning.
>  		rop->curr_recv_buf = list_entry(rimg->buf_head.next, struct rbuf, l);
>  		rop->curr_sent_buf = list_entry(rimg->buf_head.next, struct rbuf, l);
> @@ -403,8 +401,7 @@ struct roperation* handle_accept_write(
>  			pr_perror("Error preparing remote image");
>  			goto err;
>  		}
> -	}
> -	else {
> +	} else {
>  		list_del(&(rimg->l));
>  		if (flags == O_APPEND)
>  			clear_remote_image(rimg);
> @@ -510,9 +507,8 @@ struct roperation* handle_accept_cache_read(
>  		}
>  		rop_set_rimg(rop, rimg);
>  		return rop;
> -	}
> -	// The file does not exist.
> -	else if (finished_remote) {
> +	} else if (finished_remote) {
> +		// The file does not exist.
>  		pr_info("No image %s:%s.\n", path, snapshot_id);
>  		if (write_reply_header(cli_fd, ENOENT) < 0)
>  			pr_perror("Error writing reply header for unexisting image");
> @@ -628,13 +624,11 @@ void handle_local_accept(int fd)
>  	// Write/Append case (only possible in img-proxy).
>  	if (flags != O_RDONLY) {
>  		rop = handle_accept_proxy_write(cli_fd, snapshot_id, path, flags);
> -	}
> -	// Read case while restoring (img-cache).
> -	else if (restoring) {
> +	} else if (restoring) {
> +		// Read case while restoring (img-cache).
>  		rop = handle_accept_cache_read(cli_fd, snapshot_id, path, flags);
> -	}
> -	// Read case while dumping (img-proxy).
> -	else {
> +	} else {
> +		// Read case while dumping (img-proxy).
>  		rop = handle_accept_proxy_read(cli_fd, snapshot_id, path, flags);
>  	}
>  
> @@ -669,8 +663,8 @@ void finish_proxy_read(struct roperation* rop)
>  
>  		forwarding = false;
>  
> -	// If there are images waiting to be forwarded, forward the next.
> -	if (!list_empty(&rop_forwarding)) {
> +		// If there are images waiting to be forwarded, forward the next.
> +		if (!list_empty(&rop_forwarding)) {
>  			forward_remote_image(list_entry(rop_forwarding.next, struct roperation, l));
>  		}
>  	}
> @@ -682,9 +676,8 @@ void finish_proxy_write(struct roperation* rop)
>  	if (!strncmp(rop->path, DUMP_FINISH, sizeof(DUMP_FINISH))) {
>  		// TODO - couldn't we handle the DUMP_FINISH in inside handle_accept_proxy_write?
>  		finish_local();
> -	}
> -	// Normal image received, forward it.
> -	else {
> +	} else {
> +		// Normal image received, forward it.
>  		struct roperation *rop_to_forward = new_remote_operation(
>  			rop->path, rop->snapshot_id, proxy_to_cache_fd, rop->flags, false);
>  
> @@ -694,7 +687,7 @@ void finish_proxy_write(struct roperation* rop)
>  		rop_set_rimg(rop_to_forward, rop->rimg);
>  		if (list_empty(&rop_forwarding)) {
>  			forward_remote_image(rop_to_forward);
> -	}
> +		}
>  		list_add_tail(&(rop_to_forward->l), &rop_forwarding);
>  	}
>  }
> @@ -771,14 +764,11 @@ void handle_roperation(struct epoll_event *event, struct roperation *rop)
>  		// Cached side (finished receiving forwarded image)
>  		if (restoring) {
>  			finish_cache_write(rop);
> -		}
> -		// Proxy side (finished receiving local image)
> -		else {
> +		} else {
> +			// Proxy side (finished receiving local image)
>  			finish_proxy_write(rop);
>  		}
> -	}
> -	// If send operation if finished
> -	else {
> +	} else {
>  		// Proxy side (Finished forwarding image or reading it locally).
>  		if (!restoring)
>  			finish_proxy_read(rop);
> @@ -851,19 +841,17 @@ void accept_image_connections() {
>  		for (i = 0; i < n_events; i++) {
>  			// Accept from local dump/restore?
>  			if (events[i].data.ptr == &local_req_fd) {
> -				if ( events[i].events & EPOLLHUP ||
> +				if (events[i].events & EPOLLHUP ||
>  					events[i].events & EPOLLERR) {
>  					if (!finished_local)
>  						pr_perror("Unable to accept more local image connections");
>  					goto end;
>  				}
>  				handle_local_accept(local_req_fd);
> -			}
> -			else if (restoring && !forwarding && events[i].data.ptr == &proxy_to_cache_fd) {
> +			} else if (restoring && !forwarding && events[i].data.ptr == &proxy_to_cache_fd) {
>  				event_set(epoll_fd, EPOLL_CTL_DEL, proxy_to_cache_fd, 0, 0);
>  				handle_remote_accept(proxy_to_cache_fd);
> -			}
> -			else {
> +			} else {
>  				struct roperation *rop =
>  					(struct roperation*)events[i].data.ptr;
>  				event_set(epoll_fd, EPOLL_CTL_DEL, rop->fd, 0, 0);
> @@ -971,22 +959,19 @@ int64_t send_image_async(struct roperation *op)
>  				list_entry(op->curr_sent_buf->l.next, struct rbuf, l);
>  			op->curr_sent_bytes = 0;
>  			return n;
> -		}
> -		else if (op->curr_sent_bytes == op->curr_sent_buf->nbytes) {
> +		} else if (op->curr_sent_bytes == op->curr_sent_buf->nbytes) {
>  			if (close_fd)
>  				close(fd);
>  			return 0;
>  		}
>  		return n;
> -	}
> -	else if (errno == EPIPE || errno == ECONNRESET) {
> +	} else if (errno == EPIPE || errno == ECONNRESET) {
>  		pr_warn("Connection for %s:%s was closed early than expected\n",
>  			rimg->path, rimg->snapshot_id);
>  		return 0;
>  	} else if (errno == EAGAIN || errno == EWOULDBLOCK) {
>  		return errno;
> -	}
> -	else {
> +	} else {
>  		pr_perror("Write on %s:%s socket failed",
>  			rimg->path, rimg->snapshot_id);
>  		return -1;
> @@ -1014,9 +999,9 @@ int read_remote_image_connection(char *snapshot_id, char *path)
>  				path, snapshot_id);
>  		return -1;
>  	}
> -	if (!error || !strncmp(path, RESTORE_FINISH, sizeof(RESTORE_FINISH)))
> +	if (!error || !strncmp(path, RESTORE_FINISH, sizeof(RESTORE_FINISH))) {
>  		return sockfd;
> -	else if (error == ENOENT) {
> +	} else if (error == ENOENT) {
>  		pr_info("Image does not exist (%s:%s)\n", path, snapshot_id);
>  		close(sockfd);
>  		return -ENOENT;
> @@ -1184,13 +1169,13 @@ int get_curr_snapshot_id_idx(void)
>  		pull_snapshot_ids();
>  
>  	list_for_each_entry(si, &snapshot_head, l) {
> -	if (!strncmp(si->snapshot_id, snapshot_id, PATH_MAX))
> +		if (!strncmp(si->snapshot_id, snapshot_id, PATH_MAX))
>  			return idx;
>  		idx++;
>  	}
>  
>  	pr_err("Error, could not find current snapshot id (%s) fd\n",
> -			snapshot_id);
> +		snapshot_id);
>  	return -1;
>  }
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> CRIU mailing list
> CRIU@openvz.org
> https://lists.openvz.org/mailman/listinfo/criu
Pavel Tikhomirov Feb. 5, 2019, 1:58 p.m.
вт, 29 янв. 2019 г. в 20:38, Radostin Stoyanov <rstoyanov1@gmail.com>:
...

> @@ -1014,9 +999,9 @@ int read_remote_image_connection(char *snapshot_id,
> char *path)
>                                 path, snapshot_id);
>                 return -1;
>         }
> -       if (!error || !strncmp(path, RESTORE_FINISH,
> sizeof(RESTORE_FINISH)))
> +       if (!error || !strncmp(path, RESTORE_FINISH,
> sizeof(RESTORE_FINISH))) {
>                 return sockfd;
> -       else if (error == ENOENT) {
> +       } else if (error == ENOENT) {
>                 pr_info("Image does not exist (%s:%s)\n", path,
> snapshot_id);
>                 close(sockfd);
>                 return -ENOENT;


Is these hunk really an indentation fix? we have many places of:

if (condition1)
        action1;
else if (condition2) {
        action2;
        action3;
}

[snorch@snorch criu]$ git log --oneline | head -n1
109432a3d c-lib: strdup for service_address and service_binary
[snorch@snorch criu]$ git grep "^[^}]*else if.* {$" | wc -l
23
Radostin Stoyanov Feb. 5, 2019, 2:40 p.m.
On 05/02/2019 13:58, Pavel Tikhomirov wrote:
>
> вт, 29 янв. 2019 г. в 20:38, Radostin Stoyanov <rstoyanov1@gmail.com
> <mailto:rstoyanov1@gmail.com>>:
> ...
>
>     @@ -1014,9 +999,9 @@ int read_remote_image_connection(char
>     *snapshot_id, char *path)
>                                     path, snapshot_id);
>                     return -1;
>             }
>     -       if (!error || !strncmp(path, RESTORE_FINISH,
>     sizeof(RESTORE_FINISH)))
>     +       if (!error || !strncmp(path, RESTORE_FINISH,
>     sizeof(RESTORE_FINISH))) {
>                     return sockfd;
>     -       else if (error == ENOENT) {
>     +       } else if (error == ENOENT) {
>                     pr_info("Image does not exist (%s:%s)\n", path,
>     snapshot_id);
>                     close(sockfd);
>                     return -ENOENT;
>
>
> Is these hunk really an indentation fix? we have many places of:
>
Thank you for asking, this patch is actually fixing the indentation on
several places, and since
https://criu.org/How_to_submit_patches#Edit_the_source_code recommends
to follow Linux kernel coding style
<https://www.kernel.org/doc/Documentation/process/coding-style.rst>,
which states a preferred way for placing braces and spaces, and gives
the following example:

    if (x == y) {
        ..
    } else if (x > y) {
        ...
    } else {
        ....
    }

I though that it would be good to make the code consistent and follow
this practice. However, I don't have a string opinion about this and I
will be happy to send a patch to revert these changes.
> if (condition1)
>         action1;
> else if (condition2) {
>         action2;
>         action3;
> }
>
> [snorch@snorch criu]$ git log --oneline | head -n1
> 109432a3d c-lib: strdup for service_address and service_binary
> [snorch@snorch criu]$ git grep "^[^}]*else if.* {$" | wc -l
> 23
>
Pavel Tikhomirov Feb. 6, 2019, 3:12 p.m.
> > Is these hunk really an indentation fix? we have many places of:
> >
> Thank you for asking, this patch is actually fixing the indentation on
> several places, and since
> https://criu.org/How_to_submit_patches#Edit_the_source_code recommends
> to follow Linux kernel coding style
> <https://www.kernel.org/doc/Documentation/process/coding-style.rst>,
> which states a preferred way for placing braces and spaces, and gives
> the following example:
>
>     if (x == y) {
>         ..
>     } else if (x > y) {
>         ...
>     } else {
>         ....
>     }
>
> I though that it would be good to make the code consistent and follow
> this practice. However, I don't have a string opinion about this and I
> will be happy to send a patch to revert these changes.
>
>
Thanks for you explanation!

Now I see that to follow Linux coding style we actually need to fix other
23 places where we partially skip brackets only in a single branch
according to:

"Do not unnecessarily use braces where a single statement will do.
This does not apply if only one branch of a conditional statement is a
single
statement; in the latter case use braces in both branches"
https://www.kernel.org/doc/Documentation/process/coding-style.rst

So these hunk is fine.