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 handling of spaces in driver status_set() args #2801

Merged
merged 15 commits into from
Feb 14, 2025

Conversation

jimklimov
Copy link
Member

@jimklimov jimklimov commented Feb 11, 2025

Addresses part of issue #2708: cleans up some drivers to not use spaces, and for those where not easily avoidable (e.g. snmp-ups where strings come from mapping tables) tolerates them better by recursing into self for each found token (avoid duplicates).

Also fixes one bug with status_get checks added in #2565 (and maybe there's another, see below).

TODO: this is a relatively hot code path, some optimization may be useful:

  • optimize recursion to not strcmp for space again when we know it is not there, maybe use a public wrapper method vs. implem with more args, or moving this logic into a new method for use-cases where we do not pass known-clean verbatim strings
  • for status_get checks, avoid long repetitive string walks (keep an array of offsets to starts/ends of tokens in one string? keep all statuses as a dynamic array of strings and only combine into one at status_commit - also avoids sprintfcat over and over?)

TODO: Check if there's a bug about status_get stopping on first substring e.g. a contrived BYPASS BY may not report BY as having been set now. UPDATE: indeed, the bug was there. Inner and tail substrings were also impacted (PA, PASS).

@jimklimov jimklimov added the bug label Feb 11, 2025
@jimklimov jimklimov added this to the 2.8.3 milestone Feb 11, 2025
@jimklimov jimklimov added enhancement C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks labels Feb 11, 2025
@AppVeyorBot
Copy link

The fault puzzled me though: we do pre-remove '.inst' whole...

[00:36:07] CHECKING if any executable files were installed to locations other than those covered by this recipe, so might not have needed DLLs bundled near them
[00:36:08] ln: ./.inst/NUT-for-Windows-x86_64-SNAPSHOT: cannot overwrite directory
[00:36:08] Command exited with code 1

Signed-off-by: Jim Klimov <[email protected]>
Relocated code tried earlier in generic_gpio_utest.c
(not as portable a container - far from everyone builds it)

Signed-off-by: Jim Klimov <[email protected]>
Copy link
Contributor

@arnaudquette-eaton arnaudquette-eaton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AppVeyorBot
Copy link

@jimklimov jimklimov merged commit 4cdf3fd into networkupstools:master Feb 14, 2025
27 of 31 checks passed
@jimklimov jimklimov deleted the issue-2708-spaces branch February 14, 2025 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug C-str Issues and PRs about C/C++ methods, headers and data types dealing with strings and memory blocks enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants