kvs: misc cleanups #6615
Merged
kvs: misc cleanups #6615
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
rebased now that #6604 is in |
Problem: A comment about wait_create_msg_handler() says it will make a message copy from a passed in flux_msg_t. This is not true. Update the comment to indicate a message reference is taken.
Problem: Several functions that have been refactored in the past should be cleaned up as their use has clearly changed. For example, some "goto error" type paths don't actually do anything anymore. Solution: Cleanup the functions content_store_request_send() and error_event_send_to_name().
Problem: Some code in the KVS uses flux_msg_copy(). This code was written before flux_msg_incref() existed. Use flux_msg_incref() over flux_msg_copy().
Problem: The functions wait_msg_aux_set() and wait_msg_aux_get() were added to add aux data to an internally stored message copy in a wait_t. This is back when the wait structure contained a message copy created with flux_msg_copy(). However, internally flux_msg_incref() and flux_msg_decref() are now used, so these functions are a bit excessive now. Remove wait_msg_aux_set() and wait_msg_aux_get(). Instead just set aux data via flux_msg_aux_set() before passing the message to wait_create_msg_handler().
Problem: A lookup handle is passed to getroot() only because it has to be passed to getroot_request_send() in case the getroot() call will stall. This is legacy behavior from when flux_msg_copy() had to be used instead of flux_msg_incref(). Solution: Set the lookup handle as aux data in the message if we are stalling in lookup_common(). Remove the need to pass the handle as a parameter to getroot().
Problem: Throughout the KVS code errnos are preserved in cleanup paths in a manner that is not common to the rest of the flux-core codebase. Cleanup these paths by either: A) Preserve errno within appropriate "destroy" functions. B) Use ERRNO_SAFE_WRAP
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6615 +/- ##
==========================================
+ Coverage 83.78% 83.82% +0.04%
==========================================
Files 530 530
Lines 88313 88237 -76
==========================================
- Hits 73990 73968 -22
+ Misses 14323 14269 -54
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Built on top of #6604, just some misc cleanups.