Skip to content

Fix handling of spaces in driver status_set() args#2801

Merged
jimklimov merged 15 commits intonetworkupstools:masterfrom
jimklimov:issue-2708-spaces
Feb 14, 2025
Merged

Fix handling of spaces in driver status_set() args#2801
jimklimov merged 15 commits intonetworkupstools:masterfrom
jimklimov:issue-2708-spaces

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
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
… would not appear twice: delay status_set("ALARM") effect and make it reasonable [networkupstools#2801, networkupstools#2708]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
…") cases to automatically alarm_commit() if the injected value is all the alarm we have [networkupstools#2801, networkupstools#2708]

Signed-off-by: Jim Klimov <[email protected]>
jimklimov added a commit to jimklimov/nut that referenced this pull request Feb 23, 2025
…us_set("ALARM") and later proper alarm_set(), report only the content provided by the latter [networkupstools#2801, networkupstools#2708]

Signed-off-by: Jim Klimov <[email protected]>
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