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 <ptikhomirov@virtuozzo.com>
  • Loading branch information
Snorch authored and avagin committed Apr 18, 2024
1 parent 18dcf15 commit c716c4d
Showing 1 changed file with 13 additions and 14 deletions.
27 changes: 13 additions & 14 deletions criu/sk-tcp.c
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 c716c4d

Please sign in to comment.