The 'hash' subword doesn't bring any additional information. All inpcb lookup functions operate on hashes. For lookup functions that work on either exact hash or wild hash just perform s/hash_//. Rename in_pcblookup_hash() into in_pcblookup_with_lock(), emphasizing its difference to in_pcblookup_smr(). Rename in_pcblookup_hash_locked() to in_pcblookup_internal(), as it doesn't return a locked inpcb and is used only for internal purposes. Note that the IPv6 sibling of this function already lives by name in6_pcblookup_internal(). Some future changes will make such naming more justified. No functional change.
Details
Details
- Reviewers
markj pouria - Group Reviewers
network - Commits
- rG9b8eb70ca974: inpcb: improve some internal function names
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 72305 Build 69188: arc lint + arc unit
Event Timeline
Comment Actions
Thank you so much!
Since we did it for in_pcblookup_wild_locked(), we should update in6_pcblookup_hash_wild_locked too.
| sys/netinet/in_pcb.c | ||
|---|---|---|
| 2443 | Conventionally, a lookup function named foo_locked() is expected to be called with a lock held, but here that's not the case. I'd rather not break that convention. I think in_pcblookup_internal() should be called in_pcblookup_locked(). I'm not sure what to call this function though. Some ideas: in_pcblookup_with_lock(), in_pcblookup_nosmr(), in_pcblookup1(). | |
| sys/netinet/in_pcb.c | ||
|---|---|---|
| 2443 | I had such a version, too :) What is currently called in_pcblookup_internal() were to be in_pcblookup_locked. This function was in_pcblookup_hard(), but I can go with in_pcblookup_withlock(). | |
| sys/netinet/in_pcb.c | ||
|---|---|---|
| 2443 | I was about to update to the mentioned version, but then looked up into my upcoming changes. In my upcoming changes I have in_pcblookup_internal() to be called without a lock, thus it can't become in_pcblookup_locked(). The plan is that in_pcblookup_internal() will lookup the hash bucket itself, lock the bucket and return with the bucket locked. Later some consumers may grab the inpcb lock and drop bucket lock. That would be current in_pcblookup_locked(). Other consumers will see that inpcb is NULL and they are good to go with inserting their inpcb into the locked bucket. So in_pcblookup_internal() definitely stays with this name. I can rename the _locked version into: _with_lock, _withlock or _hard. | |