redfishpower: add option to test host errors #145
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: The "cycle" configs and commands have clear cut and paste errors from the off config/commands. Fix cut and paste errors.
Problem: Typical configurations are documented in redfishpower(8) except for the common header. Document common header configuration.
Problem: The tabbing / whitespace in a few functions was wrong. Fix the tabbing / whitespace.
Problem: Some variables are initialized to NULL when it's unnecessary. Do not initialize variables unless its necessary.
Problem: Code in settimeout() may index outside of array bounds. Only check if a value is non-NULL if the prior value was non-NULL.
Problem: The following terms are used interchangeably within the redfishpower code: path & url data & postdata Solution: Consistently use the term "path" to describe "paths" input by the user for configuration and use the term "postdata" for http post data. The term "url" should only be used when the full URL is constructed (i.e. when http:// is prefixed). Update terms in the manpage as well.
Problem: The item destructor was not set for the delayedcmds list. This is because items are typically moved from delayedcmds to activecmds, but there is always the chance of an error occuring. Set the item destructor for the delayedcmds list.
Problem: Globals are initialized in a init function, but we manually cleanup globals inside main(). Add a cleanup function to call when cleaning up globals.
garlick
approved these changes
Feb 22, 2024
Comment on lines
-678
to
+682
| if (av[0]) | ||
| if (av[0]) { | ||
| (*path) = xstrdup(av[0]); | ||
| if (av[1]) | ||
| (*postdata) = xstrdup(av[1]); | ||
| if (av[1]) | ||
| (*postdata) = xstrdup(av[1]); | ||
| } |
There was a problem hiding this comment.
unrelated but the parens on the LHS are unnecessary.
There was a problem hiding this comment.
ya know, I always paren these de-reference assignments ... I don't know why. Somehow makes it more clear to me that's an assignment? Or maybe I picked it up via another project.
src/redfishpower/redfishpower.c
Outdated
| @@ -125,6 +126,7 @@ static struct option longopts[] = { | |||
| {"offpostdata", required_argument, 0, 'G' }, | |||
| {"cyclepostdata", required_argument, 0, 'D' }, | |||
| {"test-mode", no_argument, 0, 'T' }, | |||
| {"test-error", required_argument, 0, 'E' }, | |||
There was a problem hiding this comment.
Suggestion: use a more descriptive option name like --test-fail-power-cmd-hosts
Problem: It would be nice to cover failures in redfishpower's test mode. When running under --test-mode, support a new --test-error option that allows users to set a group of hosts to permanently return errors when communicating with the hosts.
Problem: There is no coverage for when targetted hosts return errors. Add basic coverage in t0029-redfish.t.
|
tweaked option command per comment above, setting MWP, thanks! |
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.
Problem: It would be nice to cover failures in redfishpower's test mode.
When running under --test-mode, support a new --test-error option that allows users to set a group of hosts to permanently return errors when communicating with the hosts.
threw in a bunch of cleanups from another branch, hope that's ok