Skip to content

acceptループで1クライアントずつ処理する電卓サーバーの作成#1

Open
Fuminiton wants to merge 1 commit intomainfrom
trunk
Open

acceptループで1クライアントずつ処理する電卓サーバーの作成#1
Fuminiton wants to merge 1 commit intomainfrom
trunk

Conversation

@Fuminiton
Copy link
Copy Markdown
Owner

No description provided.

new file:   README.md
new file:   include/calculation.h
new file:   include/http_request.h
new file:   include/http_response.h
new file:   include/server.h
new file:   include/utils.h
new file:   src/core/calculation.c
new file:   src/core/http_request.c
new file:   src/core/http_response.c
new file:   src/core/server.c
new file:   src/main.c
new file:   src/utils/utils.c
new file:   tests/client.c
new file:   tests/test.sh
Copy link
Copy Markdown

@takumihara takumihara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

お疲れ様です!全体的にとても丁寧に書かれていて、他の参加者のPRでよく見る問題(ssize_tSO_REUSEADDR の値、socket 失敗時の close、エラーの区別など)が最初から対処されているのが良いですね。モジュール分割も明確で、テストも充実していて読みやすかったです。

if (!pointer) {
log_exit("Memory allocation failed for %zu bytes", size);
}
memset(pointer, 0, size);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memset するなら calloc でよさそうです


static char *server_receive_request(int client_socket) {
char *buffer = xmalloc(BUFFER_SIZE);
ssize_t bytes_read = read(client_socket, buffer, BUFFER_SIZE - 1);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read が1回で全部のデータを読んでくれるかは分からないです。\r\n\r\n がくるまでloopで回すのが正しいです

if (request->method_len != 3 || strncmp(request->method, "GET", 3) != 0) {
response_build_error(response, sizeof(response),
HTTP_BAD_REQUEST, "Only GET method is supported");
if (write(client_socket, response, strlen(response)) < 0) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

read 同様に write が全部書き込めるかは分からないです

is_shutdown_requested = 1;
}

static void server_signal_setup(void) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

せっかくなので、 SIGPIPE のハンドリングも入れてもいいかもです!

Comment on lines +47 to +48
if (write(client_socket, response, strlen(response)) < 0) {
log_error("Failed to send response: %s", strerror(errno));
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errnoEINTR の場合は、エラーログをが出さないほうが綺麗かもです

Comment on lines +22 to +23
matched = sscanf(uri_buffer, "/calc?query=%d%c%d",
out_operand1, out_op_char, out_operand2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

query が最初のquery parameterになるかは分からないですよね?

/calc?utm_source=google&query=1+2

Comment on lines +22 to +23
matched = sscanf(uri_buffer, "/calc?query=%d%c%d",
out_operand1, out_op_char, out_operand2);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

例えば、 query=2147483648+1がここにきたらどうなりますか?

log_exit("Missing required arguments.\nUsage: ./http_server <port_number>");
}

port_number = atoi(argv[1]);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

atoi("8080abc") だったらどうなりますか?

Comment on lines +61 to +63
if (argc != 3) {
printf("usage: %s [ip address] [port]\n", argv[0]);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exitし忘れですかね?

* ネットワーク処理
* ==================================================================== */

static int server_socket_create(int port) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static 関数なのでファイルスコープに閉じているため、server_ プレフィックスは不要かもしれないです。また、verb から始める命名(create_socket)にすると、何をする関数かが一目で分かりやすくなります。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants