Skip to content

Commit

Permalink
sk-tcp: cleanup dump_tcp_conn_state error handling
Browse files Browse the repository at this point in the history
1) In dump_tcp_conn_state, if return from libsoccr_save is >=0, we check
that sizeof(struct libsoccr_sk_data) returned from libsoccr_save is
equal to sizeof(struct libsoccr_sk_data) we see in dump_tcp_conn_state
(probably to check if we use the right library version). And if sizes
are different we go to err_r, which just returns ret, which can
teoretically be 0 (if size in library is zero) and that would lead
dump_one_tcp treat this as success though it is obvious error.

2) In case of dump_opt or open_image fails we don't explicitly set ret
and rely that sizeof(struct libsoccr_sk_data) previously set to ret is
not 0, I don't really like it, it makes reading code too complex.

3) We have a lot of err_* labels which do exactly the same thing, there
is no point in having all of them, also it is better to choose the name
of the label based on what it really does.

So let's refactor error handling to avoid these inconsistencies.

Signed-off-by: Pavel Tikhomirov <[email protected]>
  • Loading branch information
Snorch committed Apr 16, 2024
1 parent 1a4c103 commit 86614bc
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions criu/sk-tcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ void cpt_unlock_tcp_connections(void)
static int dump_tcp_conn_state(struct inet_sk_desc *sk)
{
struct libsoccr_sk *socr = sk->priv;
int exit_code = -1;
int ret, aux;
struct cr_img *img;
TcpStreamEntry tse = TCP_STREAM_ENTRY__INIT;
Expand All @@ -144,11 +145,11 @@ static int dump_tcp_conn_state(struct inet_sk_desc *sk)
ret = libsoccr_save(socr, &data, sizeof(data));
if (ret < 0) {
pr_err("libsoccr_save() failed with %d\n", ret);
goto err_r;
goto err;
}
if (ret != sizeof(data)) {
pr_err("This libsocr is not supported (%d vs %d)\n", ret, (int)sizeof(data));
goto err_r;
goto err;
}

sk->state = data.state;
Expand Down Expand Up @@ -190,15 +191,15 @@ static int dump_tcp_conn_state(struct inet_sk_desc *sk)
*/

if (dump_opt(sk->rfd, SOL_TCP, TCP_NODELAY, &aux))
goto err_opt;
goto err;

if (aux) {
tse.has_nodelay = true;
tse.nodelay = true;
}

if (dump_opt(sk->rfd, SOL_TCP, TCP_CORK, &aux))
goto err_opt;
goto err;

if (aux) {
tse.has_cork = true;
Expand All @@ -208,20 +209,19 @@ static int dump_tcp_conn_state(struct inet_sk_desc *sk)
/*
* Push the stuff to image
*/

img = open_image(CR_FD_TCP_STREAM, O_DUMP, sk->sd.ino);
if (!img)
goto err_img;
goto err;

ret = pb_write_one(img, &tse, PB_TCP_STREAM);
if (ret < 0)
goto err_iw;
goto err_close;

buf = libsoccr_get_queue_bytes(socr, TCP_RECV_QUEUE, SOCCR_MEM_EXCL);
if (buf) {
ret = write_img_buf(img, buf, tse.inq_len);
if (ret < 0)
goto err_iw;
goto err_close;

xfree(buf);
}
Expand All @@ -230,18 +230,17 @@ static int dump_tcp_conn_state(struct inet_sk_desc *sk)
if (buf) {
ret = write_img_buf(img, buf, tse.outq_len);
if (ret < 0)
goto err_iw;
goto err_close;

xfree(buf);
}

pr_info("Done\n");
err_iw:
exit_code = 0;
err_close:
close_image(img);
err_img:
err_opt:
err_r:
return ret;
err:
return exit_code;
}

int dump_one_tcp(int fd, struct inet_sk_desc *sk, SkOptsEntry *soe)
Expand Down

0 comments on commit 86614bc

Please sign in to comment.