assorted minor source cleanup #5918
Merged
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
Problem: czmq destructors are inconsistent with regard to 1) checking the 'arg' for NULL before dereferencing, and 2) setting '*arg' to NULL after destruction. Adjust a few functions. This probably has no effect but does improve consistency.
Problem: when run under valgrind, the libsubprocess command unit test throws the following error: ok 150 - flux_cmd_stringify on empty cmd works ==3429631== Invalid read of size 1 ==3429631== at 0x484FBD4: strcmp (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==3429631== by 0x10CAFC: mystrcmp (tap.c:100) ==3429631== by 0x10CAFC: mystrcmp (tap.c:99) ==3429631== by 0x10CAFC: is_at_loc (tap.c:110) ==3429631== by 0x10B7B4: test_stringify (command.c:226) ==3429631== by 0x10AA5B: main (command.c:459) ==3429631== Address 0x4abe1c0 is 0 bytes after a block of size 0 alloc'd ==3429631== at 0x484DA83: calloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==3429631== by 0x10DEAB: flux_cmd_stringify (command.c:484) ==3429631== by 0x10B76D: test_stringify (command.c:223) ==3429631== by 0x10AA5B: main (command.c:459) ==3429631== From malloc(3): If nmemb or size is 0, then calloc() returns either NULL, or a unique pointer value that can later be successfully passed to free(). I think when the command is empty, argz_len is zero, and calloc() returns this unique pointer, which we then dereference. Rework the function to work like the test expects.
Problem: eventlog_entry_parse() unnecessarily calls json_unpack() twice. It may not have much impact but since this function is called on performance critical paths, consolidate the calls.
Problem: broker overlay includes ccan/ptrint.h but doesn't use it. Drop #include directive.
Problem: a function call with a long parameter list is not broken to one per line, as per project norms. Fix line breaks.
Problem: autogen.sh complains of a typo. t/Makefile.am:1058: warning: variable 'job_manager_plugin_offline_la_SOURCES' is defined but no program or t/Makefile.am:1058: library has 'job_manager_plugin_offline_la' as canonical name (possible typo) Fix typo. The test plugin seems to be built anyway so this is just cleanup.
|
Hmm coverage builder got stuck here again. Restarting. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5918 +/- ##
==========================================
+ Coverage 83.29% 83.32% +0.02%
==========================================
Files 514 514
Lines 82957 82960 +3
==========================================
+ Hits 69100 69125 +25
+ Misses 13857 13835 -22
|
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.
This fixes assorted source hygiene issues, nothing major.