サーバー課題実装 by seal-azarashi · Pull Request #1 · seal-azarashi/server_assignment

seal-azarashi · 2024-10-02T06:35:40Z

レビューができるようサーバー課題の実装をプルリクエストにしています。

inazz · 2024-10-02T09:50:18Z

クライアントとサーバーで共通の問題は片方にしかコメントしていません。
バッファサイズを固定じゃなくしたいです。
また、大きな関数は読む (or メンテする)のに考えること、把握することが多くなり大変なので、
意味のある処理のまとまりを関数にまとめて名前を付けてみてください。

inazz · 2024-10-02T07:46:34Z

common_utils.h

+#define BUFFER_SIZE 1024
+#define LOOPBACK_ADDRESS "127.0.0.1"
+
+void cleanup(int socket_fd)


ヘッダファイルにはプロトタイプ宣言だけを書いて、関数の実装は .c ファイルに書くのが一般的です。
.h は複数のファイルから include されるのが想定されます。
すると、実装が複数個作られることになり、リンクの際にどれを使えばよいかわからなくなりエラーになります。

ご指摘ありがとうございます。理解しました。
同ファイルの関数はプロトタイプ宣言にし、実装を common_utils.c に記述するように修正しました。合わせて、実行コマンドが複雑になってきたので Makefile を作成しました: 9730d44

inazz · 2024-10-02T07:49:26Z

README.md

+
+## その他気になったことメモ
+
+- C では try catch で囲んでエラーが発生したら stack trace を出す、みたいなのは出来ないのだろうか？


C には例外機能は無いですが、（スタックの特定の構造を前提とした）バックトレースを取得する機能については、標準外ですが、 glibc 関数としてあったりはします。
https://www.gnu.org/software/libc/manual/html_node/Backtraces.html

inazz · 2024-10-02T07:49:34Z

client.c

+int main()
+{
+    struct sockaddr_in socket_address;
+    socket_address.sin_family = AF_INET;


IPv6 も対応するにはどうしたら良いでしょうか?

socket_address を IPv6 対応に変更し、setsockopt を使用してオプションで IPv4 も受け付けるようにしました。
affc955

inazz · 2024-10-02T07:50:07Z

client.c

+{
+    struct sockaddr_in socket_address;
+    socket_address.sin_family = AF_INET;
+    socket_address.sin_addr.s_addr = inet_addr(LOOPBACK_ADDRESS);


接続先をコマンドライン引数で指定できるようにしてみてほしい。

修正しました: 27232c2

inazz · 2024-10-02T08:13:19Z

client.c

+    if (connect(socket_fd, (struct sockaddr *)&socket_address, sizeof(socket_address)) == -1)
+    {
+        perror("connect failed");
+        cleanup(socket_fd);


cleanup, exit のコピペを減らすにはどういう構造にするとよいでしょうか?

共通処理を何かしらの手段にひとつにまとめ、各所から呼び出すようにするのはどうかと考えました。
こちらでは関数にしています: dfa7ac9

メモ: goto を使うのもよくやるらしい (このようになにかのリソースを開放するような使い方が唯一許されている goto ぐらいの例)

メモ: system call がどのような挙動をしたか等で処理を分けたくなった場合はさらに工夫が必要

inazz · 2024-10-02T08:20:34Z

client.c

+    }
+    else if (bytes_written < request_length)
+    {
+        fprintf(stderr, "Partial write occurred\n");


一部分しか write できなかったのは、失敗ではなく、想定内の挙動です。(書き込み先のネットワークのバッファが足りなかった。書き込みの途中で割り込みを受けて一部完了で復帰した、など。)
その際にも正しく処理を続けられるコードにしてみてください。

manも確認しておくと良いですね。
https://man7.org/linux/man-pages/man2/write.2.html

The number of bytes written may be less than count if, for
example, there is insufficient space on the underlying physical
medium, or the RLIMIT_FSIZE resource limit is encountered (see
setrlimit(2)), or the call was interrupted by a signal handler
after having written less than count bytes. (See also pipe(7).)

read, write 合わせてこちらで修正しました: 405644d

hayashi-ay さん、ドキュメントもありがとうございます。

サーバーについてはこちらで修正しています: 461e118

inazz · 2024-10-02T09:11:12Z

client.c

+
+    printf("Sent HTTP GET request\n");
+
+    bytes_read = read(socket_fd, buffer, BUFFER_SIZE - 1);


read も一回で全部のデータが読めるとは限りません。
（データが複数パケットに分かれて、一つ目だけ到着した場合はどうなる？)
また、受け取ったデータ全体が buffer のサイズより大きいケースも考えてください。

こちらも同様に。
https://man7.org/linux/man-pages/man2/read.2.html

It is not an error if this number is smaller than the number of
bytes requested; this may happen for example because fewer bytes
are actually available right now (maybe because we were close to
end-of-file, or because we are reading from a pipe, or from a
terminal), or because read() was interrupted by a signal.

read, write 合わせてこちらで修正しました: 405644d

サーバーについてはこちらで修正しています: 461e118

hayashi-ay · 2024-10-03T09:39:10Z

server.c

+        cleanup(socket_fd);
+        exit(EXIT_FAILURE);
+    }
+    if (listen(socket_fd, 3) == -1)


ちなみにbacklogのサイズを3にした理由はありますか？

特に深い理由はなく、ちょっと負荷を与えたらすぐに許容できなくなる程度の値にしていました。
このままでいいのかについてはスレッド導入時に再検討する予定でいますので、その旨コメントに残しておきます: 51de367

まあ決めの問題かなとは思います。3というのが意味がありそうな値だったのと、第2引数がなにか把握されているか確認してみたくらいです。

hayashi-ay · 2024-10-03T09:41:35Z

server.c

+    }
+
+    cleanup(socket_fd);
+    return 0;


EXIT_SUCCESSを使ったらどうですか？

ご指摘ありがとうございます。 global constants を把握出来ておりませんでした。こちら使うよう修正しました。
https://learn.microsoft.com/en-us/cpp/c-runtime-library/exit-success-exit-failure?view=msvc-170

良いと思います。まあ好みではあると思いますが、自分が見たときの断面ではエラー時にはEXIT_FAILUREを使っていたのが気になったのでコメントしたくらいです。

seal-azarashi · 2024-10-04T12:22:23Z

client.c

+#include <stdlib.h>
+#include <string.h>
+#include <arpa/inet.h>
+#include <errno.h>
+#include "common_utils.h"


何らかの規則性を持たせたい

alphabet 順

独自定義のファイル等と標準ライブラリの間に一行空ける

etc.

seal-azarashi · 2024-10-11T13:31:11Z

server.c

+        total_read += bytes_read;
+        if (strstr(buffer, "\r\n\r\n") != NULL) // ボディのない GET リクエストのみが来る想定
+        {
+            break;
+        }


keep-alive が来た時に変になる？
https://developer.mozilla.org/ja/docs/Web/HTTP/Headers/Keep-Alive

inazz · 2024-10-11T13:25:08Z

client.c

+    socket_address.sin6_port = htons(atoi(PORT));
+
+    char buffer[BUFFER_SIZE] = {0};
+    char *http_request = "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n";


Host の値も引数に合わせたい

inazz · 2024-10-11T13:27:33Z

client.c

+
+    printf("Sent HTTP GET request\n");
+
+    ssize_t total_received = receive_response(socket_fd, response, MAX_MESSAGE_SIZE - 1);


意味的には、
MAX_MESSAGE_SIZE = BUFFER_SIZE - 1
として MAX_MESSAGE_SIZE を渡すようにする?

seal_azarashi added 2 commits October 2, 2024 15:32

initial commit

3bf841d

fix

b3219c2

inazz reviewed Oct 2, 2024

View reviewed changes

hayashi-ay reviewed Oct 3, 2024

View reviewed changes

seal_azarashi added 7 commits October 4, 2024 17:57

fix .gitignore

98219a4

fix common util

9730d44

fix README

3e8c9e9

use global constants

ae543a2

create check_error()

dfa7ac9

fix

565e729

comment about backlog queue

51de367

seal-azarashi commented Oct 4, 2024

View reviewed changes

seal_azarashi added 11 commits October 4, 2024 22:03

fix

5f66e6f

accept ipv6 & ipv4

affc955

クライアントの送信先ホストをコマンドライン引数で指定できるようにする

27232c2

embrace partial read, write

405644d

change file name

5a17680

embrace partial read, write on server

461e118

add

ba3f545

fix typo

6845560

delete unnecessary comment

440fc53

fix comment

0171609

fix README

67a1045

seal-azarashi commented Oct 11, 2024

View reviewed changes

inazz reviewed Oct 11, 2024

View reviewed changes


		## その他気になったことメモ

		- C では try catch で囲んでエラーが発生したら stack trace を出す、みたいなのは出来ないのだろうか？


		printf("Sent HTTP GET request\n");

		bytes_read = read(socket_fd, buffer, BUFFER_SIZE - 1);


		printf("Sent HTTP GET request\n");

		ssize_t total_received = receive_response(socket_fd, response, MAX_MESSAGE_SIZE - 1);

Conversation

seal-azarashi commented Oct 2, 2024

Uh oh!

inazz commented Oct 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

seal-azarashi Oct 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

seal-azarashi Oct 4, 2024 •

edited

Loading