Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement) socket #25

Merged
merged 3 commits into from
Sep 7, 2023
Merged

implement) socket #25

merged 3 commits into from
Sep 7, 2023

Conversation

ak0327
Copy link
Owner

@ak0327 ak0327 commented Sep 6, 2023

#18 の続き

実装内容

  • branch sockets_satushiの引き継ぎ実装 99e3794
    • constructor内でsocketの作成が完了するようにした(socketを作成する手順は、constructorを呼ぶだけ、他の手続きは不要)
    • socket Socket.cpp L116
    • bind Socket.cpp L132
    • listen Socket.cpp L150
    • confの反映(とりあえずIP, PORTを直で与えることに)Socket.cpp L58-59
    • 関数の成否:Resultに格納
      • Socket classの関数はすべてnoexceptとし、関数の成否はResult型のresultに格納した
      • 呼び出し元のserverでresultをチェックし、エラーならserverでエラー文を出力し、終了する
    • private関数にしなくても良さそうな関数はlocal scopeに Socket.cpp L12
  • test 374b356

残課題など

  • test
    • socketをmaxfdまで作成するテストはうまくいかず。何個作ってもfd=3, デストラクタが走っている?
    • テストケースが弱いかも
    • macOS, Linuxで挙動が異なる点があった(一旦テストケースはコメントアウト)
      • PORT 0 : macはerror, LinuxはOK
      • PORT 65536 : macはerror, LinuxはOK(unsigned shortで0になっていそう)
      • IP: 255.255.255.255 : macはerror, LinuxはOK
  • confの受取
    • とりあえず、最低限必要なcahr *server_ip, char *server_portを受け取る形としているが、confを受け取り、参照するような変更が必要になりそう
  • confのバリデーション
    • socket作成で使うserverのIP, PORTのバリデーションはどこで?socket作成に失敗したらエラーで良いか?
    • 同様に、confのバリデーションは各項目を使うタイミングでないとチェックできない気がするが、それで良いのか?

@ak0327 ak0327 force-pushed the 17-implement-socket branch from c7fd951 to 28c448c Compare September 6, 2023 04:05
@ak0327 ak0327 force-pushed the 17-implement-socket branch from 0906d94 to 374b356 Compare September 7, 2023 00:56
@ak0327 ak0327 self-assigned this Sep 7, 2023
@ak0327 ak0327 requested review from naharagu and molhot September 7, 2023 01:08
@ak0327 ak0327 marked this pull request as ready for review September 7, 2023 01:08
Copy link
Contributor

@molhot molhot left a comment

Choose a reason for hiding this comment

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

前回とコメントとしては同じです!(前回ごめんなさい。。
オッケーです!

@molhot molhot merged commit daf8a7d into main Sep 7, 2023
@ak0327 ak0327 deleted the 17-implement-socket branch September 10, 2023 07:47
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