Fixed bug in EventProvider (Some configuration file have leading whitespaces: closes #8)

[Liteom]

Summary by CodeRabbit

• Bug Fixes

  • Improved input parsing to correctly handle leading/trailing whitespace and empty values, preventing incorrect conversions.
  • Fixed recognition and conversion of hexadecimal and decimal numeric strings for more robust event/number parsing.
  • No changes to public interfaces or behavior visible to integrations.

• Chores

  • Minor internal formatting and code cleanup with no semantic impact.

[coderabbitai]

Walkthrough

Trimmed input strings in parse_integer before parsing (removing all whitespace), adjusted hex/decimal detection to use the trimmed copy, added <cctype> include, and made a minor blank-line/whitespace formatting tweak in CsvFileEventProvider::add_events. No public APIs changed.

Changes

Cohort / File(s)	Summary
Input normalization & parsing enhancements src/event_provider.cpp	Added <cctype> include; perf::SystemSpecificEventProvider::parse_integer now copies the input to copied_str, removes all whitespace, and uses that trimmed string for emptiness checks, hex (0x) detection/parsing, and decimal digit checks; returns std::nullopt on empty or failed parse. Also minor blank-line/whitespace formatting around CsvFileEventProvider::add_events when reading the event name.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I nibbled spaces left and right,

I hopped through digits, bold and bright,
Hex and dec now find their place,
Trimmed and tidy—swift the race! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Fixed bug in EventProvider (Some configuration file have leading whitespaces: closes #8)" directly and accurately describes the main change in the changeset. The PR modifies the EventProvider to trim whitespace from parsed input values, specifically addressing the issue where configuration files with leading whitespaces were not being handled correctly. The title is specific enough that a teammate scanning the commit history would immediately understand this is about fixing whitespace handling in event provider configuration parsing, and it ties the change to a specific issue number for traceability.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

src/event_provider.cpp (2)

336-337: LGTM! Consider applying this pattern to similar code.

The whitespace trimming approach correctly addresses the issue with leading/trailing whitespace in configuration files. The use of unsigned char in the lambda avoids undefined behavior with std::isspace for characters with negative values.

However, lines 220-221 use ::isspace directly without the safer unsigned char cast pattern. Consider updating them to match this implementation:

key.erase(std::remove_if(key.begin(), key.end(), [](unsigned char c){return std::isspace(c);}), key.end()); value.erase(std::remove_if(value.begin(), value.end(), [](unsigned char c){return std::isspace(c);}), value.end());

---

344-350: Add exception handling for robustness.

std::stoull can throw std::out_of_range if the number exceeds UINT64_MAX. While unlikely with system-generated configuration files, adding exception handling would make the function more consistent with its std::optional return type contract, which implies graceful failure handling.

Consider wrapping the std::stoull calls in try-catch blocks:

 /// Strings starting with '0x' are considered hex numbers. if (copied_str.rfind("0x", 0ULL) == 0ULL) { - return std::stoull(copied_str.substr(2ULL), nullptr, 16); + try { + return std::stoull(copied_str.substr(2ULL), nullptr, 16); + } catch (const std::out_of_range&) { + return std::nullopt; + } } /// Strings containing digits are considered dec numbers. if (std::all_of(copied_str.begin(), copied_str.end(), [](const auto c) { return std::isdigit(c); })) { - return std::stoull(copied_str, nullptr, 0); + try { + return std::stoull(copied_str, nullptr, 0); + } catch (const std::out_of_range&) { + return std::nullopt; + } }

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 75e5f62 and 6fe9634.

📒 Files selected for processing (1)

• src/event_provider.cpp (3 hunks)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/event_provider.cpp (1)

335-353: Whitespace trimming correctly fixes the reported bug.

The approach of creating a trimmed copy and using it for all parsing operations is sound and resolves the issue with leading whitespace in configuration files.

However, for consistency and maximum safety, consider using unsigned char in the lambda at line 348, matching line 336:

Apply this diff for type consistency:

- if (std::all_of(copied_str.begin(), copied_str.end(), [](const auto c) { return std::isdigit(c); })) { + if (std::all_of(copied_str.begin(), copied_str.end(), [](unsigned char c) { return std::isdigit(c); })) {

This ensures both lambdas follow the same pattern and avoids potential undefined behavior with std::isdigit on platforms where char is signed.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6fe9634 and 2c2f8b6.

📒 Files selected for processing (1)

• src/event_provider.cpp (4 hunks)

🔇 Additional comments (2)

src/event_provider.cpp (2)

13-13: LGTM: Include addition is correct.

The <cctype> include is necessary for std::isspace and std::isdigit used in the parse_integer function.

---

489-490: LGTM: Formatting change has no semantic impact.

[jmuehlig]

Thanks for the bugfix. I will add it to the v0.12.3 release.