The unp_connect_peer() is used only once and the contract with
unp_connectat() was always hard to understand. Inlining, we got isolation
of UNP_CONNECTING flag and also more clear understanding when peer socket
reference needs to be released, and when not. Also, reading linear
sequence of the operation is easier than jumping across two functions.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 78489 Build 75372: arc lint + arc unit
Event Timeline
Comment Actions
This is what I wrote @glebius and @markj while Phabricator was down
Verdict: "can't complain" :)
As much as I prefer smaller functions / smaller scopes / structuring the error handling, I have to admit this was not as "bad" as I feared. The function that you *kept* (unp_connectat_peer [1]) is the function that I really like, and
that would be called an additional time in my patch stack. In contrast, the function that you inlined (unp_connect_peer) is always called just once throughout all my patches. So yes, I cannot actually justify it with "but it will be useful later" via patches I already wrote.[2][2]: Off the top of my head, one use-case would be to support a new system call that returns a fresh socket for the connection, and therefore can skip the "is this socket ready to be connected" steps at the beginning. But I am fine simply reproposing factoring out this function if I ever actually get around to putting up a patch for such a new system call. (And I think I would work on the forkless process spawning patch series first. :))