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

Fix some unitialized fields in client struct #1126

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

rjd15372
Copy link
Contributor

@rjd15372 rjd15372 commented Oct 4, 2024

This commit adds initialization code for the fields io_last_reply_block and io_last_bufpos of the client struct.

While in the current code flow, these fields are only accessed after being written in the trySendWriteToIOThreads, I discovered that they were not being initialized while doing some changes to the code flow of IO threads.

I believe it's good pratice to initialize all fields of a struct upon creation, and will avoid future bugs which are usually hard to debug.

This commit adds initialization code for the fields
`io_last_reply_block` and `io_last_bufpos` of the `client` struct.

While in the current code flow, these fields are only accessed after
being written in the `trySendWriteToIOThreads`, I discovered that they
were not being initialized while doing some changes to the code flow of
IO threads.

I believe it's good pratice to initialize all fields of a struct upon
creation, and will avoid future bugs which are usually hard to debug.

Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Copy link

codecov bot commented Oct 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.62%. Comparing base (dcac3e1) to head (c0ecc30).
Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1126      +/-   ##
============================================
+ Coverage     70.61%   70.62%   +0.01%     
============================================
  Files           114      114              
  Lines         61711    61713       +2     
============================================
+ Hits          43576    43585       +9     
+ Misses        18135    18128       -7     
Files with missing lines Coverage Δ
src/networking.c 88.00% <100.00%> (-0.42%) ⬇️

... and 15 files with indirect coverage changes

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.

Yeah, I agree it's better to initialize everything. It's likely free from a performance perspective as well since these operations are fast.

@madolson madolson merged commit 6a8540c into valkey-io:unstable Oct 4, 2024
47 checks passed
naglera pushed a commit to naglera/placeholderkv that referenced this pull request Oct 10, 2024
This commit adds initialization code for the fields
`io_last_reply_block` and `io_last_bufpos` of the `client` struct.

While in the current code flow, these fields are only accessed after
being written in the `trySendWriteToIOThreads`, I discovered that they
were not being initialized while doing some changes to the code flow of
IO threads.

I believe it's good pratice to initialize all fields of a struct upon
creation, and will avoid future bugs which are usually hard to debug.

Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
Signed-off-by: naglera <anagler123@gmail.com>
SoftlyRaining pushed a commit to SoftlyRaining/valkey that referenced this pull request Oct 11, 2024
This commit adds initialization code for the fields
`io_last_reply_block` and `io_last_bufpos` of the `client` struct.

While in the current code flow, these fields are only accessed after
being written in the `trySendWriteToIOThreads`, I discovered that they
were not being initialized while doing some changes to the code flow of
IO threads.

I believe it's good pratice to initialize all fields of a struct upon
creation, and will avoid future bugs which are usually hard to debug.

Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
eifrah-aws pushed a commit to eifrah-aws/valkey that referenced this pull request Oct 20, 2024
This commit adds initialization code for the fields
`io_last_reply_block` and `io_last_bufpos` of the `client` struct.

While in the current code flow, these fields are only accessed after
being written in the `trySendWriteToIOThreads`, I discovered that they
were not being initialized while doing some changes to the code flow of
IO threads.

I believe it's good pratice to initialize all fields of a struct upon
creation, and will avoid future bugs which are usually hard to debug.

Signed-off-by: Ricardo Dias <ricardo.dias@percona.com>
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.

2 participants