settings: shell: improve reading and writing string values #85786
Conversation
Make reading and writing string values more flexible: 1. Eliminate the intermediate buffer when saving a string setting. This needlessly limited the maximum string length that could be saved using the shell command. 2. Do not add nor assume that a string saved in the settings includes the null-terminator. The settings subsystem uses metadata for encoding the value length, so there it is redundant to also store the null-terminator in flash. By the way, also make sure the command handlers only return -EINVAL and -ENOEXEC error codes as documented in the handler type description. Signed-off-by: Damian Krolik <damian.krolik@nordicsemi.no>
|
@danielstuart14 Let me know if you're OK with changing the behavior that you added. If not, I can keep adding the null-terminator in the write command :). |
LGTM! |
| @@ -138,7 +143,7 @@ static int cmd_read(const struct shell *shell_ptr, size_t argc, char *argv[]) | |||
| err = settings_parse_type(argv[1], &value_type); | |||
| if (err) { | |||
| shell_error(shell_ptr, "Invalid type: %s", argv[1]); | |||
| return err; | |||
| return -EINVAL; | |||
There was a problem hiding this comment.
but settings_parse_type already returns -EINVAL, we should just propagate the error
There was a problem hiding this comment.
Yes, but this makes it a bit clearer that the handler does not return any other error code than the two allowed.
| enum settings_value_types value_type = SETTINGS_VALUE_HEX; | ||
|
|
||
| if (argc > 3) { | ||
| err = settings_parse_type(argv[1], &value_type); | ||
| if (err) { | ||
| shell_error(shell_ptr, "Invalid type: %s", argv[1]); | ||
| return err; | ||
| return -EINVAL; |
| shell_error(shell_ptr, "Setting not found"); | ||
| err = -ENOEXEC; |
There was a problem hiding this comment.
By the way, also make sure the command handlers only return
-EINVAL and -ENOEXEC error codes as documented in the
handler type description.
Can you point me to this documentation ?
There was a problem hiding this comment.
| } | ||
| shell_print(params->shell_ptr, "%s", buffer); | ||
| shell_print(params->shell_ptr, "%.*s", (int)num_read_bytes, buffer); |
Make reading and writing string values more flexible:
By the way, also make sure the command handlers only return -EINVAL and -ENOEXEC error codes as documented in the handler type description.