From 2e5fb403abaec0e48d7c0a5b7dcc435cab0a134d Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 16:48:42 +0800 Subject: [PATCH 1/7] refactor(capi): prefer early returns for `read_cb` --- capi/examples/client.c | 27 +++++++++++++-------------- capi/examples/upload.c | 27 +++++++++++++-------------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/capi/examples/client.c b/capi/examples/client.c index 3cccdd6ae1..9550e5545b 100644 --- a/capi/examples/client.c +++ b/capi/examples/client.c @@ -24,22 +24,21 @@ static size_t read_cb(void *userdata, hyper_context *ctx, uint8_t *buf, size_t b struct conn_data *conn = (struct conn_data *)userdata; ssize_t ret = read(conn->fd, buf, buf_len); - if (ret < 0) { - int err = errno; - if (err == EAGAIN) { - // would block, register interest - if (conn->read_waker != NULL) { - hyper_waker_free(conn->read_waker); - } - conn->read_waker = hyper_context_waker(ctx); - return HYPER_IO_PENDING; - } else { - // kaboom - return HYPER_IO_ERROR; - } - } else { + if (ret >= 0) { return ret; } + + if (errno != EAGAIN) { + // kaboom + return HYPER_IO_ERROR; + } + + // would block, register interest + if (conn->read_waker != NULL) { + hyper_waker_free(conn->read_waker); + } + conn->read_waker = hyper_context_waker(ctx); + return HYPER_IO_PENDING; } static size_t write_cb(void *userdata, hyper_context *ctx, const uint8_t *buf, size_t buf_len) { diff --git a/capi/examples/upload.c b/capi/examples/upload.c index 10582c8867..66469fac0b 100644 --- a/capi/examples/upload.c +++ b/capi/examples/upload.c @@ -24,22 +24,21 @@ static size_t read_cb(void *userdata, hyper_context *ctx, uint8_t *buf, size_t b struct conn_data *conn = (struct conn_data *)userdata; ssize_t ret = read(conn->fd, buf, buf_len); - if (ret < 0) { - int err = errno; - if (err == EAGAIN) { - // would block, register interest - if (conn->read_waker != NULL) { - hyper_waker_free(conn->read_waker); - } - conn->read_waker = hyper_context_waker(ctx); - return HYPER_IO_PENDING; - } else { - // kaboom - return HYPER_IO_ERROR; - } - } else { + if (ret >= 0) { return ret; } + + if (errno != EAGAIN) { + // kaboom + return HYPER_IO_ERROR; + } + + // would block, register interest + if (conn->read_waker != NULL) { + hyper_waker_free(conn->read_waker); + } + conn->read_waker = hyper_context_waker(ctx); + return HYPER_IO_PENDING; } static size_t write_cb(void *userdata, hyper_context *ctx, const uint8_t *buf, size_t buf_len) { From 95e86bd53c8c62bf64d5fdba1036f8288bbb297a Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 16:51:58 +0800 Subject: [PATCH 2/7] refactor(capi): prefer early returns for `write_cb` --- capi/examples/client.c | 27 +++++++++++++-------------- capi/examples/upload.c | 27 +++++++++++++-------------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/capi/examples/client.c b/capi/examples/client.c index 9550e5545b..899ee29381 100644 --- a/capi/examples/client.c +++ b/capi/examples/client.c @@ -45,22 +45,21 @@ static size_t write_cb(void *userdata, hyper_context *ctx, const uint8_t *buf, s struct conn_data *conn = (struct conn_data *)userdata; ssize_t ret = write(conn->fd, buf, buf_len); - if (ret < 0) { - int err = errno; - if (err == EAGAIN) { - // would block, register interest - if (conn->write_waker != NULL) { - hyper_waker_free(conn->write_waker); - } - conn->write_waker = hyper_context_waker(ctx); - return HYPER_IO_PENDING; - } else { - // kaboom - return HYPER_IO_ERROR; - } - } else { + if (ret >= 0) { return ret; } + + if (errno != EAGAIN) { + // kaboom + return HYPER_IO_ERROR; + } + + // would block, register interest + if (conn->write_waker != NULL) { + hyper_waker_free(conn->write_waker); + } + conn->write_waker = hyper_context_waker(ctx); + return HYPER_IO_PENDING; } static void free_conn_data(struct conn_data *conn) { diff --git a/capi/examples/upload.c b/capi/examples/upload.c index 66469fac0b..8e4ecb1896 100644 --- a/capi/examples/upload.c +++ b/capi/examples/upload.c @@ -45,22 +45,21 @@ static size_t write_cb(void *userdata, hyper_context *ctx, const uint8_t *buf, s struct conn_data *conn = (struct conn_data *)userdata; ssize_t ret = write(conn->fd, buf, buf_len); - if (ret < 0) { - int err = errno; - if (err == EAGAIN) { - // would block, register interest - if (conn->write_waker != NULL) { - hyper_waker_free(conn->write_waker); - } - conn->write_waker = hyper_context_waker(ctx); - return HYPER_IO_PENDING; - } else { - // kaboom - return HYPER_IO_ERROR; - } - } else { + if (ret >= 0) { return ret; } + + if (errno != EAGAIN) { + // kaboom + return HYPER_IO_ERROR; + } + + // would block, register interest + if (conn->write_waker != NULL) { + hyper_waker_free(conn->write_waker); + } + conn->write_waker = hyper_context_waker(ctx); + return HYPER_IO_PENDING; } static void free_conn_data(struct conn_data *conn) { From 851bee7aed243ecee9d6a0b682f730095e2a2787 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 16:53:02 +0800 Subject: [PATCH 3/7] refactor(capi): remove redundant `else` after break Since the `if` condition already causes the loop to `break`, the `else` is not necessary. We wouldn't have reached the `else` block, anyway, if the prior `if` condition passed. --- capi/examples/client.c | 4 ++-- capi/examples/upload.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/capi/examples/client.c b/capi/examples/client.c index 899ee29381..cb73716781 100644 --- a/capi/examples/client.c +++ b/capi/examples/client.c @@ -96,9 +96,9 @@ static int connect_to(const char *host, const char *port) { if (connect(sfd, rp->ai_addr, rp->ai_addrlen) != -1) { break; - } else { - close(sfd); } + + close(sfd); } freeaddrinfo(result); diff --git a/capi/examples/upload.c b/capi/examples/upload.c index 8e4ecb1896..75ec614b3e 100644 --- a/capi/examples/upload.c +++ b/capi/examples/upload.c @@ -96,9 +96,9 @@ static int connect_to(const char *host, const char *port) { if (connect(sfd, rp->ai_addr, rp->ai_addrlen) != -1) { break; - } else { - close(sfd); } + + close(sfd); } freeaddrinfo(result); From 08d6664e1766d48452145c29d27281be181633b1 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 16:56:21 +0800 Subject: [PATCH 4/7] format(capi): fix some irregular spacing an indentation --- capi/examples/client.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/capi/examples/client.c b/capi/examples/client.c index cb73716781..be62102978 100644 --- a/capi/examples/client.c +++ b/capi/examples/client.c @@ -140,17 +140,17 @@ typedef enum { #define STR_ARG(XX) (uint8_t *)XX, strlen(XX) int main(int argc, char *argv[]) { - const char *host = argc > 1 ? argv[1] : "httpbin.org"; - const char *port = argc > 2 ? argv[2] : "80"; - const char *path = argc > 3 ? argv[3] : "/"; - printf("connecting to port %s on %s...\n", port, host); + const char *host = argc > 1 ? argv[1] : "httpbin.org"; + const char *port = argc > 2 ? argv[2] : "80"; + const char *path = argc > 3 ? argv[3] : "/"; + printf("connecting to port %s on %s...\n", port, host); - int fd = connect_to(host, port); + int fd = connect_to(host, port); if (fd < 0) { return 1; } - printf("connected to %s, now get %s\n", host, path); + printf("connected to %s, now get %s\n", host, path); if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) { printf("failed to set socket to non-blocking\n"); return 1; @@ -166,7 +166,6 @@ int main(int argc, char *argv[]) { conn->read_waker = NULL; conn->write_waker = NULL; - // Hookup the IO hyper_io *io = hyper_io_new(); hyper_io_set_userdata(io, (void *)conn); From e95a75845ce55002104ace3e53101a9a0f62f4aa Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 16:57:29 +0800 Subject: [PATCH 5/7] refactor(capi): remove unneeded `else` block --- capi/examples/client.c | 19 ++++++++++--------- capi/examples/upload.c | 18 +++++++++--------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/capi/examples/client.c b/capi/examples/client.c index be62102978..57a3e7b6c7 100644 --- a/capi/examples/client.c +++ b/capi/examples/client.c @@ -312,15 +312,16 @@ int main(int argc, char *argv[]) { if (sel_ret < 0) { printf("select() error\n"); return 1; - } else { - if (FD_ISSET(conn->fd, &fds_read)) { - hyper_waker_wake(conn->read_waker); - conn->read_waker = NULL; - } - if (FD_ISSET(conn->fd, &fds_write)) { - hyper_waker_wake(conn->write_waker); - conn->write_waker = NULL; - } + } + + if (FD_ISSET(conn->fd, &fds_read)) { + hyper_waker_wake(conn->read_waker); + conn->read_waker = NULL; + } + + if (FD_ISSET(conn->fd, &fds_write)) { + hyper_waker_wake(conn->write_waker); + conn->write_waker = NULL; } } diff --git a/capi/examples/upload.c b/capi/examples/upload.c index 75ec614b3e..299b76d69d 100644 --- a/capi/examples/upload.c +++ b/capi/examples/upload.c @@ -385,17 +385,17 @@ int main(int argc, char *argv[]) { if (sel_ret < 0) { printf("select() error\n"); return 1; - } else { - if (FD_ISSET(conn->fd, &fds_read)) { - hyper_waker_wake(conn->read_waker); - conn->read_waker = NULL; - } - if (FD_ISSET(conn->fd, &fds_write)) { - hyper_waker_wake(conn->write_waker); - conn->write_waker = NULL; - } } + if (FD_ISSET(conn->fd, &fds_read)) { + hyper_waker_wake(conn->read_waker); + conn->read_waker = NULL; + } + + if (FD_ISSET(conn->fd, &fds_write)) { + hyper_waker_wake(conn->write_waker); + conn->write_waker = NULL; + } } From 9e1bf4fd005e97122c8dfdc0c83d40dbd5781833 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 17:21:13 +0800 Subject: [PATCH 6/7] refactor(capi): reorder result check for `poll_req_upload` We are more likely to poll a successful chunk than finish the request or throw an error. Thus, it is best if we go for the optimistic route and check for the successful case first. --- capi/examples/upload.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/capi/examples/upload.c b/capi/examples/upload.c index 299b76d69d..fdefd903e9 100644 --- a/capi/examples/upload.c +++ b/capi/examples/upload.c @@ -124,17 +124,20 @@ static int poll_req_upload(void *userdata, struct upload_body* upload = userdata; ssize_t res = read(upload->fd, upload->buf, upload->len); - if (res < 0) { - printf("error reading upload file: %d", errno); - return HYPER_POLL_ERROR; - } else if (res == 0) { + if (res > 0) { + *chunk = hyper_buf_copy(upload->buf, res); + return HYPER_POLL_READY; + } + + if (res == 0) { // All done! *chunk = NULL; return HYPER_POLL_READY; - } else { - *chunk = hyper_buf_copy(upload->buf, res); - return HYPER_POLL_READY; } + + // Oh no! + printf("error reading upload file: %d", errno); + return HYPER_POLL_ERROR; } static int print_each_header(void *userdata, From b272ee9d7baf8484c262e37ef102487c00ec2224 Mon Sep 17 00:00:00 2001 From: Basti Ortiz <39114273+Some-Dood@users.noreply.github.com> Date: Mon, 11 Apr 2022 17:24:23 +0800 Subject: [PATCH 7/7] refactor(capi): remove another unneeded `else` after `break` --- capi/examples/upload.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/capi/examples/upload.c b/capi/examples/upload.c index fdefd903e9..9b791eedf8 100644 --- a/capi/examples/upload.c +++ b/capi/examples/upload.c @@ -349,20 +349,20 @@ int main(int argc, char *argv[]) { hyper_executor_push(exec, body_data); break; - } else { - assert(task_type == HYPER_TASK_EMPTY); - hyper_task_free(task); - hyper_body_free(resp_body); + } - printf("\n -- Done! -- \n"); + assert(task_type == HYPER_TASK_EMPTY); + hyper_task_free(task); + hyper_body_free(resp_body); - // Cleaning up before exiting - hyper_executor_free(exec); - free_conn_data(conn); - free(upload.buf); + printf("\n -- Done! -- \n"); - return 0; - } + // Cleaning up before exiting + hyper_executor_free(exec); + free_conn_data(conn); + free(upload.buf); + + return 0; case EXAMPLE_NOT_SET: // A background task for hyper completed... hyper_task_free(task);