Skip to content

Replace magic number with named constant in format_time bounds check#30

Merged
Nikoro merged 4 commits intodevelopfrom
copilot/sub-pr-29
Nov 24, 2025
Merged

Replace magic number with named constant in format_time bounds check#30
Nikoro merged 4 commits intodevelopfrom
copilot/sub-pr-29

Conversation

Copy link

Copilot AI commented Nov 24, 2025

Addresses code review feedback on PR #29 regarding unclear magic number 15 in bounds checking logic.

Changes:

  • Replaced magic number 15 with MAX_REPLACEMENT_LENGTH = 10 constant in format_time()
  • Added inline comment documenting that "hh:mm:ss a" is the longest replacement (10 chars)
  • Updated bounds check to BUFFER_LENGTH - MAX_REPLACEMENT_LENGTH - 1 for clarity
// Before
while (time[i] != '\0' && pos < BUFFER_LENGTH - 15) {

// After  
const int MAX_REPLACEMENT_LENGTH = 10; // "hh:mm:ss a" is the longest
while (time[i] != '\0' && pos < BUFFER_LENGTH - MAX_REPLACEMENT_LENGTH - 1) {

The new constant is less conservative (11 vs 15) but correctly sized for the actual longest replacement string.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits November 24, 2025 19:50
Co-authored-by: Nikoro <12560977+Nikoro@users.noreply.github.com>
Co-authored-by: Nikoro <12560977+Nikoro@users.noreply.github.com>
Copilot AI changed the title [WIP] Address feedback on AM/PM pattern conversion Replace magic number with named constant in format_time bounds check Nov 24, 2025
Copilot AI requested a review from Nikoro November 24, 2025 19:53
@Nikoro Nikoro marked this pull request as ready for review November 24, 2025 19:55
@Nikoro Nikoro requested a review from Copilot November 24, 2025 19:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses code review feedback by replacing a magic number with a named constant to improve code clarity in the format_time() function's bounds checking logic.

  • Introduced MAX_REPLACEMENT_LENGTH constant to replace the magic number 15
  • Added documentation explaining that "hh:mm:ss a" is the longest replacement string
  • Updated the bounds check calculation to use the new constant

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
linux/system_date_time_format_plugin.cc Replaced magic number 15 with MAX_REPLACEMENT_LENGTH = 10 constant and updated bounds check logic
_codeql_detected_source_root Added CodeQL configuration file (metadata change)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Nikoro Nikoro merged commit 9c7c3fa into develop Nov 24, 2025
@Nikoro Nikoro deleted the copilot/sub-pr-29 branch November 24, 2025 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants