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.

Sign up for GitHub

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

Jump to bottom

Part five: Doorman rewrite #54

Merged
merged 81 commits into from
Aug 2, 2020
Merged

Part five: Doorman rewrite #54

merged 81 commits into from
Aug 2, 2020

Conversation

mattgodbolt
Copy link
Owner

@mattgodbolt mattgodbolt commented Jul 28, 2020 •
edited
Loading

Now ready, based off #83 now to hopefully show the actual differences.

Previous TODOs:

  • Tests
  • Fixing the god-awful input parsing and telnet option handling.
  • Run some fuzzing/bad behaving clients over it
  • Coding convention renames
  • Channel to no longer be a static array (and probably use non-repeating channel IDs to get around some race conditions I've noted). Channels written so construction initialises everything and all the horrid pointers-instead-of-reference and magic numbers in newConnection can go. CC @philjessies for the comment he made about the crimes committed in newConnection
  • Replace log_out (with maybe spdlog or just fmt::print(stderr...))
  • Replace char buffer[4096] and similar with some max-sized payload constant, shared between MUD and doorman. See @snellers comment about MaxIncoming
  • grep for TODO, e.g. the writev stuff.

@mattgodbolt
Doorman c++ first step
57c6f52 
Get compiling and working with no actual changes to layout or behaviour.
@mattgodbolt
Merge remote-tracking branch 'origin/main' into mg/doorman
a8a3e09
@mattgodbolt
Bring in fmtlib, remove some strdups. First steps to c++ification
a053c95
@mattgodbolt
Replace "ident" lookup with a std::stringy version
0359072
@mattgodbolt
NULL->nullptr
0c2655c
@mattgodbolt
Staged update, slowly moving towards a more clean solution.
069912a
@mattgodbolt
Character name is a std::string
4942bd8
@mattgodbolt
Remove the buffer's mallocyness
550e419
@mattgodbolt
Beginnings of OOification, pending testabilities
c23831a
@mattgodbolt
Oops bug!
3754937
@mattgodbolt
Merge remote-tracking branch 'origin/main' into mg/doorman
552d980
@mattgodbolt
Enable the address and undefined sanitizers for debug builds.
974c363
@mattgodbolt
Rename some confusing globals
31a7432
@mattgodbolt
Fix missing includes
330d859
@mattgodbolt
More extraction and OOification
eb895b2
@mattgodbolt
Sanitize C too (thanks @snellers)
3691216
@mattgodbolt
Class-i-fication of Channel.
51d1843
@mattgodbolt
Merge remote-tracking branch 'origin/main' into mg/doorman
59a6e28
@mattgodbolt
Use width/height
2df5f5c
@mattgodbolt
Reformat
5a1f969
@mattgodbolt
Fix reconnect message
940634a
@mattgodbolt
Kill the ifds global variable.
8e2b58c
@mattgodbolt
First attempt at class extraction.
90cc73b 
Honestly, it would have been more sane to have just rewritten everything.
@mattgodbolt
Extract an FD RAII class. Add GSl
8e6f731
@mattgodbolt
Reformat
3b27cbe
@mattgodbolt
Merge remote-tracking branch 'origin/main' into mg/doorman
a474105
@mattgodbolt
Death to all globals!!!
6dc786f
@mattgodbolt
More sanity in pipe FDs
cedad3e
@mattgodbolt mattgodbolt added the enhancement New feature or request label Jul 28, 2020
snellers
snellers reviewed Jul 28, 2020
View reviewed changes
src/doorman/Channel.cpp Outdated Show resolved Hide resolved
@mattgodbolt mattgodbolt changed the title Doorman rewrite Part five: Doorman rewrite Aug 1, 2020
@mattgodbolt mattgodbolt marked this pull request as ready for review August 1, 2020 15:53
snellers
snellers reviewed Aug 2, 2020
View reviewed changes
src/doorman/Doorman.cpp Outdated Show resolved Hide resolved
snellers
snellers reviewed Aug 2, 2020
View reviewed changes
src/comm.c Show resolved Hide resolved
snellers
snellers reviewed Aug 2, 2020
View reviewed changes
src/doorman/Channel.cpp Outdated Show resolved Hide resolved
mattgodbolt added a commit that referenced this pull request Aug 2, 2020
@mattgodbolt
Part one of doorman PR (see #54) (#80)
179d011 
* Part one of doorman PR (see #54)
* Introduce a file descriptor C++ wrapper.
* Add a header to define byte
mattgodbolt added a commit that referenced this pull request Aug 2, 2020
@mattgodbolt
Part two of doorman PR (see #54) (#81)
f1efe85 
Builds on #80

* Introduce a standalone, tested telnet protocol handler.
mattgodbolt added a commit that referenced this pull request Aug 2, 2020
@mattgodbolt
Part three of doorman PR (see #54) (#82)
3b47176 
Builds on #81

A general purpose ID allocator. The allocator gives out small integer identifiers, along with a reference-counted reservation that holds them as "in use".

IDs are eagerly recycled once the reference count hits zero.

Useful for Xania and doorman to both have a "vote" for which channels are in-use. Previously we had bugs where there was a race in doorman if someone disconnected while xania was about to send a ton of info while another connection came in. The new connection would be given the channel ID of the recently-closed connection and would thus be delivered all the messages for the old connection.
mattgodbolt added a commit that referenced this pull request Aug 2, 2020
@mattgodbolt
Part four of doorman PR (see #54) (#83)
c658d8e 
* Part four of doorman PR (see #54)

Builds on #82

Adds a logger, based on spdlog.
Base automatically changed from mg/doorman_log to main August 2, 2020 13:44
@mattgodbolt
Merge remote-tracking branch 'origin/main' into mg/doorman
5798c4f
@mattgodbolt
Remove duplicated line
99e5bce
@mattgodbolt
Extract unique_ptr<> (per @snellers comment about hard to find channe…
cf6ea60 
…l construction)
@mattgodbolt
Fix brain-dead bug with unconnected MUD
e08b89e
@mattgodbolt
Rephrase write_to_descriptor.
e905210 
 Now uses PACKET_MAX_PAYLOAD_SIZE to prevent ever sending more than we can handle over the xania<->doorman protocol.

Fixes SendPacket to actually check the right things and return the right value.

Per discussion with @snellers I think this handles the xania->doorman side of things. The doorman->xania and buffer handling on the xania side I think should be fixed as part of #46.
@mattgodbolt
Copy link
Owner Author

Thanks all: will merge and be damned after I get back from being out. I'll then babysit an upgrade and make sure all works out ok!

@mattgodbolt mattgodbolt merged commit 0df3453 into main Aug 2, 2020
@mattgodbolt mattgodbolt deleted the mg/doorman branch August 2, 2020 23:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snellers snellers snellers approved these changes

@philjessies philjessies Awaiting requested review from philjessies

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
3 participants
@mattgodbolt @snellers @philjessies