クライアント・サーバーの実装課題 #1

TORUS0818 · 2024-10-03T10:14:00Z

一旦動作確認が出来たのでPRを出します。

他の方々のPRのコメントを確認しましたが、

IPv6やホスト名での指定への対応
closeやexitの集約
適度な関数切り出し

などに課題を感じました。

C言語に慣れておらず、都度キャッチアップしながら書いているので、おかしなところが多々あるかもしれません。
お作法等も併せてコメント頂けるとありがたいです。

hayashi-ay

各システムコールと実際のTCP通信のフローがどう対応しているかなどは確認しておくと良いかなと思います。three way hand shakingはどれなど。必ずしもシステムコールと1対1で対応しているわけではないですが。

あとはrecv, send周りの処理は他の方のPRのコメントも参考にされると良いと思います。

あとは今のサーバーの実装はブロッキングになっている（1度に1つのクライアントとしか通信できない）と思うのでノンブロッキングにするとかですかね。selectシステムコールあたりを見てみると良いと思います。

hayashi-ay · 2024-10-03T10:17:46Z

client.c

+#define BUFFER_SIZE (MESSAGE_SIZE + 1)
+
+int main(int argc, char* argv[]) {
+    int sd;


あまり見ない命名な気がします。file descripterなのでfdとかですかね？

socket descripterのつもりでした。

hayashi-ay · 2024-10-03T10:20:22Z

client.c

+        fprintf(stderr, "invalid IP address.\n");
+        exit(EXIT_FAILURE);
+    }
+    if ((server_port = (unsigned short) atoi(argv[2])) == 0) {


atoiよりかはstrotolの方がoverflowとかも分かるので良いかなと思います。あとはポート番号はもう少しバリデーションしても良いかなと思います。ex. wellknow portは使わせないなど https://en.wikipedia.org/wiki/List_of_TCP_and_UDP_port_numbers

https://man7.org/linux/man-pages/man3/strtol.3.html

参考資料もありがとうございます！

TORUS0818 · 2024-10-03T14:18:22Z

ご指摘いただいた点を意識して、もう少し色々調べてみます！

inazz

server/client 共通の話は片方にのみコメントしています。
IPv6に対応してみてください。
全てをメイン関数に書かず、適当な処理単位に分割し関数にして、名前を付けてください。
リモートからは任意のデータが飛んでくる可能性があると想定してください。最大長を仮定しないでください。

inazz · 2024-10-03T10:54:56Z

client.c

+        fprintf(stderr, "argument count missmatch.\n");
+        exit(EXIT_FAILURE);
+    }
+    if (inet_aton(argv[1], &server_sa.sin_addr) == 0) {


ip v6 や、ホスト名が指定されても通信できるようにしてほしい。

以下で修正しました。
0b9dca6

inazz · 2024-10-03T10:56:43Z

client.c

+    }
+
+    memset(&server_sa, 0, sizeof(server_sa));
+    server_sa.sin_family = AF_INET;


これだと IPv4 のみになってしまうので、 v6 でも対応できるようにしたい。

以下で修正しました。
0b9dca6

inazz · 2024-10-03T10:57:16Z

client.c

+    char receive_buffer[BUFFER_SIZE], send_buffer[BUFFER_SIZE];
+
+    if (argc != 3) {
+        fprintf(stderr, "argument count missmatch.\n");


使う人にわかりやすいエラーメッセージを考えてみてください。

以下で修正しました。
752536b

inazz · 2024-10-03T22:18:24Z

client.c

+
+    while(1) {
+        printf("please enter the characters: ");
+        if (fgets(send_buffer, BUFFER_SIZE, stdin) == NULL) {


入力の終了に対応してください (EOF -- 一般的には、端末で Ctrl-D を押した場合)

以下で修正しました。
fc9b5be

inazz · 2024-10-03T22:20:05Z

client.c

+            exit(EXIT_FAILURE);
+        }
+        // send
+        if (send(sd, send_buffer, strlen(send_buffer), 0) <= 0) {


一部だけ送信に成功したケースを考慮してください。

inazz · 2024-10-03T23:10:08Z

client.c

+        int current_byte = 0;
+        while(current_byte < BUFFER_SIZE) {
+            // recv
+            received_byte = recv(sd, &receive_buffer[current_byte], 1, 0);


システムコールを呼び出すと、
CPU割込みが発生し、権限を切り替え、ユーザの権限やパラメータチェックをし、必要に応じてロックを取ったりと、、、と、データを１バイトコピーするよりはるかに大きな労力がかかります。
まとまった単位で読み込むようにしてください。

以下で修正しました。
fc9b5be

inazz · 2024-10-03T23:10:38Z

client.c

+            if (received_byte < 0) {
+                perror("recv() failed.\n");
+                exit(EXIT_FAILURE);
+            } else if (received_byte == 0) {


正常に最後までデータ後届いた場合もエラーになってしまっています。

以下で修正しました。
fc9b5be

inazz · 2024-10-03T23:14:22Z

client.c

+
+            if (receive_buffer[current_byte] == '\n') {
+                receive_buffer[current_byte] = '\0';
+                if (strcmp(receive_buffer, "quit") == 0) {


receive_buffer に \0 が入っていない場合はバッファオーバーフローの危険性があります。
(実際には第2引数の5バイト目が\0なので起きないだろうとは推測しますが、strcmp は保証してくれない)

以下で修正しました。（上記処理を廃止しました。）
fc9b5be

inazz · 2024-10-03T23:55:43Z

server.c

+
+        while(1) {
+            //recv
+            if ((receive_message_size = recv(client_sd, receive_buffer, BUFFER_SIZE, 0)) < 0) {


recv も同様に一回で全部読めるとは保証されません。
また、先方から送られてくるデータがBUFFER_SIZEより多い場合はどうなるでしょう?

送られてきたデータがバッファより大きい場合には、動的にバッファを拡大して対応できるように実装できそうです。（対応予定）

inazz · 2024-10-03T23:59:46Z

client.c

+        printf("received from server: %s\n", receive_buffer);
+    }
+    return EXIT_SUCCESS;
+}


一般的にテキストファイルの末尾は改行を入れることになっています。

以下で修正しました。
ee4e10c

TORUS0818 · 2024-10-09T09:52:28Z

残課題メモ：

ノンブロッキング対応(poll or selectを使う)
初期バッファサイズを超えるデータ受信時の対応

inazz · 2024-10-11T11:53:06Z

client.c

@@ -0,0 +1,88 @@
+#include <stdio.h>


#include の順番に何か規則を持たせたい。 (<> -> "" の順番で、アルファベット順など)

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

common.c

+        n = send(s, buf + sent_total, len - sent_total, 0);
+        if (n == -1) { 
+            perror("send() failed.\n");
+            exit(EXIT_FAILURE);


もし失敗時に exit するなら、 send_all_or_die みたいな名前にしたい。

inazz · 2024-10-11T12:02:54Z

common.c

+        }
+        if (n == 0) {
+            printf("end of input detected (EOF).\n");
+            return sent_total;


send_all なので、 paritial に終わらせたくない。
(partial な時に exit を呼ぶならエラー時と同じ挙動でまだ理解しやすいかも)

inazz · 2024-10-11T12:05:49Z

common.c

+            return received_total;
+        }
+        received_total += n;
+        if (strchr(buf, '\n') != NULL) {


receive_all と書いてあるので、データの内容については触れたくない。

inazz · 2024-10-11T12:07:13Z

client.c

+            printf("server disconnected.\n");
+            break;
+        }
+        receive_buffer[receive_message_size] = '\0';


受け取ったデータがバッファ全部に詰まっていた際に、これはバッファの次のメモリ (out of bounds) になってしまう。

inazz · 2024-10-11T12:12:26Z

server.c

+
+        while(1) {
+            //recv
+            receive_message_size = receive_all(client_sd, receive_buffer, BUFFER_SIZE);


receive_all はエラー時に exit するけれど、
client の一つが通信エラー発生しただけでサーバーを終了させたくない。

望月佳彦 and others added 4 commits October 3, 2024 11:42

Add: client.cを追加

d32a997

Add: server.cを追加

8db084b

Update README.md

f554a06

Create memo.md

b735988

hayashi-ay reviewed Oct 3, 2024

View reviewed changes

inazz reviewed Oct 4, 2024

View reviewed changes

TORUS0818 force-pushed the dev branch from ec65bca to b735988 Compare October 5, 2024 14:36

望月佳彦 and others added 6 commits October 7, 2024 11:04

Clean: 引数に関するエラーメッセージの修正

752536b

Clean: 改行追加

ee4e10c

Update: プロトコル非依存な実装に修正

0b9dca6

Clean: 表記の修正

4423a0b

Update memo.md

2cf0ad8

Update: 送受信の制御を修正

fc9b5be

望月佳彦 added 2 commits October 9, 2024 21:57

Clean: 変数名の修正

3dc20fa

Fix: addrinfoを解放する処理を追加

6dcf304

TORUS0818 force-pushed the dev branch from dbc10cd to 6dcf304 Compare October 10, 2024 10:44

Update memo.md

213f588

inazz reviewed Oct 11, 2024

View reviewed changes

クライアント・サーバーの実装課題 #1

Are you sure you want to change the base?

クライアント・サーバーの実装課題 #1

Uh oh!

Conversation

TORUS0818 commented Oct 3, 2024

Uh oh!

hayashi-ay left a comment

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!

TORUS0818 commented Oct 3, 2024

Uh oh!

inazz left a comment

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!

TORUS0818 Oct 9, 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!

TORUS0818 Oct 9, 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!

TORUS0818 commented Oct 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

TORUS0818 Oct 9, 2024 •

edited

Loading

TORUS0818 Oct 9, 2024 •

edited

Loading

TORUS0818 commented Oct 9, 2024 •

edited

Loading