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

Introduce connection context for Unix socket #1160

Merged

Conversation

pizhenwei
Copy link
Contributor

Hide 'unixsocketgroup' and 'unixsocketperm' into a Unix socket specific data structure. A single opaque pointer 'void *priv' is enough for a listener. Once any new config is added, we don't need 'void *priv2', 'void *priv3' and so on.

Fixes: b48596a ('Add support for setting the group on a unix domain socket (#901)')
Cc: Ayush Sharma [email protected]

@pizhenwei pizhenwei force-pushed the introduce-unix-connection-context branch 2 times, most recently from 86b62e2 to 0235b6b Compare October 13, 2024 09:38
Copy link

codecov bot commented Oct 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.70%. Comparing base (a62d1f1) to head (acf811f).
Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1160      +/-   ##
============================================
+ Coverage     70.65%   70.70%   +0.05%     
============================================
  Files           114      114              
  Lines         61799    63073    +1274     
============================================
+ Hits          43664    44597     +933     
- Misses        18135    18476     +341     
Files with missing lines Coverage Δ
src/config.c 78.84% <ø> (+0.14%) ⬆️
src/connection.h 87.20% <ø> (+0.46%) ⬆️
src/server.c 88.74% <100.00%> (+0.10%) ⬆️
src/server.h 100.00% <ø> (ø)
src/unix.c 76.25% <100.00%> (-0.07%) ⬇️

... and 88 files with indirect coverage changes

Hide 'unixsocketgroup' and 'unixsocketperm' into a Unix socket
specific data structure. A single opaque pointer 'void *priv' is
enough for a listener. Once any new config is added, we don't need
'void *priv2', 'void *priv3' and so on.

Fixes: b48596a ('Add support for setting the group on a unix domain socket (valkey-io#901)')
Cc: Ayush Sharma <[email protected]>
Signed-off-by: zhenwei pi <[email protected]>
Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

This was a decision made at the time (for simplicity), #901 (review)

i am ok with both, i will let @madolson do the call.

Copy link
Member

@madolson madolson left a comment

Choose a reason for hiding this comment

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

This seems reasonable to me. I didn't think of this solution at the time.

@madolson madolson merged commit 64cfdf6 into valkey-io:unstable Oct 19, 2024
47 checks passed
@pizhenwei pizhenwei deleted the introduce-unix-connection-context branch October 19, 2024 04:40
eifrah-aws pushed a commit to eifrah-aws/valkey that referenced this pull request Oct 20, 2024
Hide 'unixsocketgroup' and 'unixsocketperm' into a Unix socket specific
data structure. A single opaque pointer 'void *priv' is enough for a
listener. Once any new config is added, we don't need 'void *priv2',
'void *priv3' and so on.

Signed-off-by: zhenwei pi <[email protected]>
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.

3 participants