Skip to content

Commit 1d895b8

Browse files
BastiDoodBastiDood
authored
refactor(capi): make early returns consistent in C examples (#2812)
- 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. - 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.
1 parent e3ee1de commit 1d895b8

File tree

2 files changed

+102
-103
lines changed

2 files changed

+102
-103
lines changed

‎capi/examples/client.c‎

Lines changed: 44 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -24,44 +24,42 @@ static size_t read_cb(void *userdata, hyper_context *ctx, uint8_t *buf, size_t b
2424
struct conn_data *conn = (struct conn_data *)userdata;
2525
ssize_t ret = read(conn->fd, buf, buf_len);
2626

27-
if (ret < 0) {
28-
int err = errno;
29-
if (err == EAGAIN) {
30-
// would block, register interest
31-
if (conn->read_waker != NULL) {
32-
hyper_waker_free(conn->read_waker);
33-
}
34-
conn->read_waker = hyper_context_waker(ctx);
35-
return HYPER_IO_PENDING;
36-
} else {
37-
// kaboom
38-
return HYPER_IO_ERROR;
39-
}
40-
} else {
27+
if (ret >= 0) {
4128
return ret;
4229
}
30+
31+
if (errno != EAGAIN) {
32+
// kaboom
33+
return HYPER_IO_ERROR;
34+
}
35+
36+
// would block, register interest
37+
if (conn->read_waker != NULL) {
38+
hyper_waker_free(conn->read_waker);
39+
}
40+
conn->read_waker = hyper_context_waker(ctx);
41+
return HYPER_IO_PENDING;
4342
}
4443

4544
static size_t write_cb(void *userdata, hyper_context *ctx, const uint8_t *buf, size_t buf_len) {
4645
struct conn_data *conn = (struct conn_data *)userdata;
4746
ssize_t ret = write(conn->fd, buf, buf_len);
4847

49-
if (ret < 0) {
50-
int err = errno;
51-
if (err == EAGAIN) {
52-
// would block, register interest
53-
if (conn->write_waker != NULL) {
54-
hyper_waker_free(conn->write_waker);
55-
}
56-
conn->write_waker = hyper_context_waker(ctx);
57-
return HYPER_IO_PENDING;
58-
} else {
59-
// kaboom
60-
return HYPER_IO_ERROR;
61-
}
62-
} else {
48+
if (ret >= 0) {
6349
return ret;
6450
}
51+
52+
if (errno != EAGAIN) {
53+
// kaboom
54+
return HYPER_IO_ERROR;
55+
}
56+
57+
// would block, register interest
58+
if (conn->write_waker != NULL) {
59+
hyper_waker_free(conn->write_waker);
60+
}
61+
conn->write_waker = hyper_context_waker(ctx);
62+
return HYPER_IO_PENDING;
6563
}
6664

6765
static void free_conn_data(struct conn_data *conn) {
@@ -98,9 +96,9 @@ static int connect_to(const char *host, const char *port) {
9896

9997
if (connect(sfd, rp->ai_addr, rp->ai_addrlen) != -1) {
10098
break;
101-
} else {
102-
close(sfd);
10399
}
100+
101+
close(sfd);
104102
}
105103

106104
freeaddrinfo(result);
@@ -142,17 +140,17 @@ typedef enum {
142140
#define STR_ARG(XX) (uint8_t *)XX, strlen(XX)
143141

144142
int main(int argc, char *argv[]) {
145-
const char *host = argc > 1 ? argv[1] : "httpbin.org";
146-
const char *port = argc > 2 ? argv[2] : "80";
147-
const char *path = argc > 3 ? argv[3] : "/";
148-
printf("connecting to port %s on %s...\n", port, host);
143+
const char *host = argc > 1 ? argv[1] : "httpbin.org";
144+
const char *port = argc > 2 ? argv[2] : "80";
145+
const char *path = argc > 3 ? argv[3] : "/";
146+
printf("connecting to port %s on %s...\n", port, host);
149147

150-
int fd = connect_to(host, port);
148+
int fd = connect_to(host, port);
151149
if (fd < 0) {
152150
return 1;
153151
}
154-
printf("connected to %s, now get %s\n", host, path);
155152

153+
printf("connected to %s, now get %s\n", host, path);
156154
if (fcntl(fd, F_SETFL, O_NONBLOCK) != 0) {
157155
printf("failed to set socket to non-blocking\n");
158156
return 1;
@@ -168,7 +166,6 @@ int main(int argc, char *argv[]) {
168166
conn->read_waker = NULL;
169167
conn->write_waker = NULL;
170168

171-
172169
// Hookup the IO
173170
hyper_io *io = hyper_io_new();
174171
hyper_io_set_userdata(io, (void *)conn);
@@ -315,15 +312,16 @@ int main(int argc, char *argv[]) {
315312
if (sel_ret < 0) {
316313
printf("select() error\n");
317314
return 1;
318-
} else {
319-
if (FD_ISSET(conn->fd, &fds_read)) {
320-
hyper_waker_wake(conn->read_waker);
321-
conn->read_waker = NULL;
322-
}
323-
if (FD_ISSET(conn->fd, &fds_write)) {
324-
hyper_waker_wake(conn->write_waker);
325-
conn->write_waker = NULL;
326-
}
315+
}
316+
317+
if (FD_ISSET(conn->fd, &fds_read)) {
318+
hyper_waker_wake(conn->read_waker);
319+
conn->read_waker = NULL;
320+
}
321+
322+
if (FD_ISSET(conn->fd, &fds_write)) {
323+
hyper_waker_wake(conn->write_waker);
324+
conn->write_waker = NULL;
327325
}
328326

329327
}

‎capi/examples/upload.c‎

Lines changed: 58 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -24,44 +24,42 @@ static size_t read_cb(void *userdata, hyper_context *ctx, uint8_t *buf, size_t b
2424
struct conn_data *conn = (struct conn_data *)userdata;
2525
ssize_t ret = read(conn->fd, buf, buf_len);
2626

27-
if (ret < 0) {
28-
int err = errno;
29-
if (err == EAGAIN) {
30-
// would block, register interest
31-
if (conn->read_waker != NULL) {
32-
hyper_waker_free(conn->read_waker);
33-
}
34-
conn->read_waker = hyper_context_waker(ctx);
35-
return HYPER_IO_PENDING;
36-
} else {
37-
// kaboom
38-
return HYPER_IO_ERROR;
39-
}
40-
} else {
27+
if (ret >= 0) {
4128
return ret;
4229
}
30+
31+
if (errno != EAGAIN) {
32+
// kaboom
33+
return HYPER_IO_ERROR;
34+
}
35+
36+
// would block, register interest
37+
if (conn->read_waker != NULL) {
38+
hyper_waker_free(conn->read_waker);
39+
}
40+
conn->read_waker = hyper_context_waker(ctx);
41+
return HYPER_IO_PENDING;
4342
}
4443

4544
static size_t write_cb(void *userdata, hyper_context *ctx, const uint8_t *buf, size_t buf_len) {
4645
struct conn_data *conn = (struct conn_data *)userdata;
4746
ssize_t ret = write(conn->fd, buf, buf_len);
4847

49-
if (ret < 0) {
50-
int err = errno;
51-
if (err == EAGAIN) {
52-
// would block, register interest
53-
if (conn->write_waker != NULL) {
54-
hyper_waker_free(conn->write_waker);
55-
}
56-
conn->write_waker = hyper_context_waker(ctx);
57-
return HYPER_IO_PENDING;
58-
} else {
59-
// kaboom
60-
return HYPER_IO_ERROR;
61-
}
62-
} else {
48+
if (ret >= 0) {
6349
return ret;
6450
}
51+
52+
if (errno != EAGAIN) {
53+
// kaboom
54+
return HYPER_IO_ERROR;
55+
}
56+
57+
// would block, register interest
58+
if (conn->write_waker != NULL) {
59+
hyper_waker_free(conn->write_waker);
60+
}
61+
conn->write_waker = hyper_context_waker(ctx);
62+
return HYPER_IO_PENDING;
6563
}
6664

6765
static void free_conn_data(struct conn_data *conn) {
@@ -98,9 +96,9 @@ static int connect_to(const char *host, const char *port) {
9896

9997
if (connect(sfd, rp->ai_addr, rp->ai_addrlen) != -1) {
10098
break;
101-
} else {
102-
close(sfd);
10399
}
100+
101+
close(sfd);
104102
}
105103

106104
freeaddrinfo(result);
@@ -126,17 +124,20 @@ static int poll_req_upload(void *userdata,
126124
struct upload_body* upload = userdata;
127125

128126
ssize_t res = read(upload->fd, upload->buf, upload->len);
129-
if (res < 0) {
130-
printf("error reading upload file: %d", errno);
131-
return HYPER_POLL_ERROR;
132-
} else if (res == 0) {
127+
if (res > 0) {
128+
*chunk = hyper_buf_copy(upload->buf, res);
129+
return HYPER_POLL_READY;
130+
}
131+
132+
if (res == 0) {
133133
// All done!
134134
*chunk = NULL;
135135
return HYPER_POLL_READY;
136-
} else {
137-
*chunk = hyper_buf_copy(upload->buf, res);
138-
return HYPER_POLL_READY;
139136
}
137+
138+
// Oh no!
139+
printf("error reading upload file: %d", errno);
140+
return HYPER_POLL_ERROR;
140141
}
141142

142143
static int print_each_header(void *userdata,
@@ -348,20 +349,20 @@ int main(int argc, char *argv[]) {
348349
hyper_executor_push(exec, body_data);
349350

350351
break;
351-
} else {
352-
assert(task_type == HYPER_TASK_EMPTY);
353-
hyper_task_free(task);
354-
hyper_body_free(resp_body);
352+
}
355353

356-
printf("\n -- Done! -- \n");
354+
assert(task_type == HYPER_TASK_EMPTY);
355+
hyper_task_free(task);
356+
hyper_body_free(resp_body);
357357

358-
// Cleaning up before exiting
359-
hyper_executor_free(exec);
360-
free_conn_data(conn);
361-
free(upload.buf);
358+
printf("\n -- Done! -- \n");
362359

363-
return 0;
364-
}
360+
// Cleaning up before exiting
361+
hyper_executor_free(exec);
362+
free_conn_data(conn);
363+
free(upload.buf);
364+
365+
return 0;
365366
case EXAMPLE_NOT_SET:
366367
// A background task for hyper completed...
367368
hyper_task_free(task);
@@ -387,17 +388,17 @@ int main(int argc, char *argv[]) {
387388
if (sel_ret < 0) {
388389
printf("select() error\n");
389390
return 1;
390-
} else {
391-
if (FD_ISSET(conn->fd, &fds_read)) {
392-
hyper_waker_wake(conn->read_waker);
393-
conn->read_waker = NULL;
394-
}
395-
if (FD_ISSET(conn->fd, &fds_write)) {
396-
hyper_waker_wake(conn->write_waker);
397-
conn->write_waker = NULL;
398-
}
399391
}
400392

393+
if (FD_ISSET(conn->fd, &fds_read)) {
394+
hyper_waker_wake(conn->read_waker);
395+
conn->read_waker = NULL;
396+
}
397+
398+
if (FD_ISSET(conn->fd, &fds_write)) {
399+
hyper_waker_wake(conn->write_waker);
400+
conn->write_waker = NULL;
401+
}
401402
}
402403

403404

0 commit comments

Comments
 (0)