feat: サーバ・クライアントの実装 by Satorien · Pull Request #1 · Satorien/Socket-Programming

Satorien · 2026-01-01T10:35:25Z

READMEに記載の仕様に基づいて実装

Copilot

Pull request overview

This PR implements a basic TCP socket-based server-client communication system in C. The server listens on port 8080 and handles arithmetic calculation requests via a /calc HTTP endpoint, while the client sends calculation queries and displays responses.

Key changes:

Utility functions for socket initialization, error handling, and logging
Server implementation with signal handling for graceful shutdown and request processing
Client implementation with command-line argument support for sending calculation requests

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
utils.h	Declares utility functions for socket operations, error handling, and logging
utils.c	Implements utility functions for socket initialization, cleanup, error handling, and logging
server.c	Implements TCP server with HTTP-like request handling for arithmetic calculations
client.c	Implements TCP client that connects to server and sends calculation requests
Makefile	Build configuration for compiling server and client executables
.gitignore	Ignores compiled executables with run_* pattern

Critical Issues Found:

Multiple memory leaks in both server and client code
Missing error checking on read/write operations
Incorrect Content-Length in HTTP responses
Security vulnerabilities including buffer overflow risks and unvalidated input
Inappropriate error handling that terminates the server on malformed requests
Signal handler with incorrect function signature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

server.c

client.c

server.c

client.c

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

utils.h

server.c

client.c

server.c

Copilot · 2026-01-02T15:17:51Z

@Satorien I've opened a new pull request, #2, to work on those changes. Once the pull request is ready, I'll request review from you.

Satorien · 2026-01-02T15:25:23Z

変更は不要と判断したため、PRはクローズしました

akmhmgc

全体的に読みやすかったです！
不正な入力などの細かい対応は今後されると思うのでコメントはしていません。

akmhmgc · 2026-01-03T00:16:30Z

client.c

+void connect_client_to_server(int client_socket_fd) {
+    // サーバアドレス構造体
+    struct sockaddr_in server_address;
+    int domain = (strcmp(IP_VERSION, "IPv6") == 0) ? AF_INET6 : AF_INET;


address familyなので変数名はdomainではなくaddress_family等が良いと思いました。

akmhmgc · 2026-01-03T00:31:25Z

client.c

+    while (sent_len < request_len) {
+        ssize_t n = write(client_socket_fd, request + sent_len, request_len - sent_len);
+        if (n < 0) {
+            handle_error("Write to server failed");
+        }
+        sent_len += n;
+    }


short count対策しているのが良いと思いました！

(nits) サーバー側でも同様の処理をしているのでラッパー関数を書いても良いかもしれません。

akmhmgc · 2026-01-03T00:34:01Z

utils.c

+void close_socket(int socket_fd) {
+    // ソケットのクローズ
+    if (close(socket_fd) < 0) {
+        handle_error("Close failed");
+    }
+}


ソケットクローズ時のエラー対応しているの良いですね！

akmhmgc · 2026-01-03T00:40:32Z

client.c

+    size_t sent_len = 0;
+    while (sent_len < request_len) {
+        ssize_t n = write(client_socket_fd, request + sent_len, request_len - sent_len);
+        if (n < 0) {


EINTRはハンドラの処理が終わった後復帰する可能性があるのでリトライしても良いかもしれません。

akmhmgc · 2026-01-03T00:48:39Z

client.c

+    size_t request_len = strlen(request);
+    size_t sent_len = 0;
+    while (sent_len < request_len) {
+        ssize_t n = write(client_socket_fd, request + sent_len, request_len - sent_len);


今だとサーバーのソケットが閉じている時にこのループでwriteが二回以上呼び出されると、SIGPIPEを受け取ってクライアントのプロセスが落ちると思います。
SIGPIPEを無視するにはwriteではなくてflagを利用できるsendを利用すると良いです。
ref: https://linuxjm.sourceforge.io/html/LDP_man-pages/man2/send.2.html

akmhmgc · 2026-01-03T03:52:52Z

server.c

+void handle_client_request(int client_socket_fd) {
+    // リクエストの処理
+    char *buffer = calloc(BUFFER_SIZE, 1);
+    ssize_t bytes_read = read(client_socket_fd, buffer, BUFFER_SIZE - 1);


writeと同様readもshort countの可能性があるのでループして要求した文字数分読めたかチェックした方が良さそうです。

akmhmgc · 2026-01-03T03:59:07Z

server.c

+    char *buffer = calloc(BUFFER_SIZE, 1);
+    ssize_t bytes_read = read(client_socket_fd, buffer, BUFFER_SIZE - 1);
+    if (bytes_read < 0) {
+        log_msg("Read from client failed\n");


ログを出して後続の処理に行ってしまうのでreturnした方が良いと思います。

akmhmgc · 2026-01-03T04:01:59Z

server.c

+    // アドレスの再利用を有効化
+    int optval = 1;
+    if (setsockopt(server_socket, SOL_SOCKET, SO_REUSEADDR, &optval, sizeof(optval)) == -1) {
+        log_msg("setsockopt SO_REUSEADDR failed");


エラーを見る限り致命的なものしかないので、ここで失敗した場合は処理を終了した方が良い気がします。
https://linuxjm.sourceforge.io/html/LDP_man-pages/man2/setsockopt.2.html

akmhmgc · 2026-01-03T04:12:35Z

server.c

+    }
+
+    // リクエストの受信と処理
+    while (is_server_running) {


シグナルハンドラでフラグを変更してgraceful shutdownに対応しているは良いですね。参考になりました。

akmhmgc · 2026-01-03T04:18:49Z

server.c

+        size_t response_len = strlen(response);
+        size_t sent_len = 0;
+        while (sent_len < response_len) {
+            ssize_t n = write(client_socket_fd, response + sent_len, response_len - sent_len);


https://github.com/Satorien/Socket-Programming/pull/1/changes#r2658611560
と同様にクライアントがソケットを閉じた後にwriteが呼び出されるとサーバーがクラッシュするのでsendでオプションを渡すのが良いと思います。

feat: サーバ・クライアントの実装

ffd8400

Satorien requested a review from Copilot January 1, 2026 10:36

Copilot started reviewing on behalf of Satorien January 1, 2026 10:36 View session

Copilot AI reviewed Jan 1, 2026

View reviewed changes

Satorien added 2 commits January 2, 2026 15:51

fix: error handling

f188991

feat: bad request response

e7a22dd

Satorien requested a review from Copilot January 2, 2026 15:01

Copilot started reviewing on behalf of Satorien January 2, 2026 15:02 View session

Copilot AI reviewed Jan 2, 2026

View reviewed changes

Copilot AI mentioned this pull request Jan 2, 2026

clarify: Explain why ternary operator correctly selects address family constants #2

Closed

fix: minor format

174b38b

akmhmgc reviewed Jan 3, 2026

View reviewed changes

Conversation

Satorien commented Jan 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Jan 2, 2026

Uh oh!

Satorien commented Jan 2, 2026

Uh oh!

akmhmgc 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!

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!

4 participants