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

Remove TLSCONN_DEBUG dead code in tls.c #1891

Merged

Conversation

WelongZuo
Copy link
Contributor

@WelongZuo WelongZuo commented Mar 27, 2025

This code is a dead code, there is also a undefined fd error in it.
Obviously this will not compile if this kind of debugging is enabled.

This seems to be broken ever since 5a47794
which was done in 2019. It's dead code that we never test and never use.

Fixes #1887

@WelongZuo WelongZuo force-pushed the fix_undefined_var_in_tlsHandleEvent branch 2 times, most recently from d91ba50 to eb9f06b Compare March 27, 2025 01:53
@enjoy-binbin
Copy link
Member

Can you explain how this is triggered? Does it only happen in debug mode?

@WelongZuo WelongZuo closed this Mar 27, 2025
@WelongZuo WelongZuo reopened this Mar 27, 2025
Copy link

codecov bot commented Mar 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.01%. Comparing base (06990c2) to head (16de41b).
Report is 4 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1891      +/-   ##
============================================
+ Coverage     71.00%   71.01%   +0.01%     
============================================
  Files           123      123              
  Lines         65671    65673       +2     
============================================
+ Hits          46631    46639       +8     
+ Misses        19040    19034       -6     
Files with missing lines Coverage Δ
src/tls.c 100.00% <ø> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@WelongZuo
Copy link
Contributor Author

Can you explain how this is triggered? Does it only happen in debug mode?

I haven't trigger this bug in production enviroment. Just find it when i learn the code.
From the define in tls.c maybe only when build with marco "TLS_DEBUGGING" can run it.
I think it's a very small problem that we can fix or ignore.

@WelongZuo WelongZuo force-pushed the fix_undefined_var_in_tlsHandleEvent branch from eb9f06b to 16de41b Compare March 29, 2025 07:58
@enjoy-binbin enjoy-binbin changed the title fix undefined variable 'fd' in tlsHandleEvent Remove TLSCONN_DEBUG dead code in tls.c Mar 29, 2025
@enjoy-binbin enjoy-binbin merged commit da32443 into valkey-io:unstable Mar 29, 2025
48 checks passed
ranshid pushed a commit to ranshid/valkey that referenced this pull request Mar 31, 2025
This code is a dead code, there is also a undefined fd error in it.
Obviously this will not compile if this kind of debugging is enabled.

This seems to be broken ever since
5a47794
which was done in 2019. It's dead code that we never test and never use.

Fixes valkey-io#1887

Signed-off-by: WelongZuo <[email protected]>
Co-authored-by: z00636558 <[email protected]>
@WelongZuo WelongZuo deleted the fix_undefined_var_in_tlsHandleEvent branch March 31, 2025 08:01
ranshid pushed a commit that referenced this pull request Mar 31, 2025
This code is a dead code, there is also a undefined fd error in it.
Obviously this will not compile if this kind of debugging is enabled.

This seems to be broken ever since
5a47794
which was done in 2019. It's dead code that we never test and never use.

Fixes #1887

Signed-off-by: WelongZuo <[email protected]>
Co-authored-by: z00636558 <[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.

Undefined variable 'fd'
3 participants