Skip to content

Refactor counter wrap-around logic into helper function#1

Draft
Copilot wants to merge 10 commits intodevelopfrom
copilot/apply-feedback-changes
Draft

Refactor counter wrap-around logic into helper function#1
Copilot wants to merge 10 commits intodevelopfrom
copilot/apply-feedback-changes

Conversation

Copy link

Copilot AI commented Feb 17, 2026

Addresses feedback from PR #902: duplicated wrap-around delta logic and platform-dependent counter width.

Changes

  • Helper function: calculate_counter_delta(uint32_t current, uint32_t previous) - static inline, uses unsigned arithmetic's natural modular wrap-around
  • Platform-independent: uint32_t instead of unsigned long ensures consistent 32-bit semantics regardless of platform word size
  • Applied to 9 counters: BytesSent, BytesReceived, PacketsSent, PacketsReceived, ErrorsSent, RetransCount, FailedRetransCount, RetryCount, MultipleRetryCount
  • Local scope: Variables declared within usage blocks

Implementation

static inline uint32_t calculate_counter_delta(uint32_t current, uint32_t previous)
{
    // Unsigned arithmetic naturally handles wrap-around correctly
    // Example: 0x00000001 - 0xFFFFFFFF = 2 (in uint32_t)
    return current - previous;
}

// Usage
uint32_t curr = sta[i].dev_stats.cli_BytesSent;
uint32_t prev = dev_stats_last->cli_BytesSent;
uint32_t diff = calculate_counter_delta(curr, prev);

Previous approach used explicit wrap-around calculation with ULONG_MAX, which is 64-bit on 64-bit platforms, causing incorrect delta computation when counters wrap at 32-bit boundary.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 8 commits February 17, 2026 10:35
- Created calculate_counter_delta() helper function to eliminate duplicated wrap-around logic
- Uses UINT32_MAX instead of ULONG_MAX for platform-independent 32-bit counter handling
- Applied helper function to all counter calculations (BytesSent/Received, PacketsSent/Received, ErrorsSent, RetransCount, FailedRetransCount, RetryCount, MultipleRetryCount)
- Addresses PR rdkcentral#902 feedback about code duplication and counter width handling

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
- Changed calculate_counter_delta() to use uint32_t instead of unsigned long for platform independence
- Moved variable declarations to local scope within each counter calculation block
- Added explicit casts to unsigned long for snprintf format compatibility

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
- Removed explicit wrap-around calculation since unsigned arithmetic naturally handles it
- Simplified calculate_counter_delta() to just return current - previous
- Updated documentation to reflect this approach

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
- Added detailed explanation of how unsigned arithmetic handles wrap-around
- Included example case to clarify the mechanism

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
- Enhanced explanation to clarify that the delta represents counter increments
- Made it clear that the example shows the counter wrapping from max to 0 to 1

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
- Clarified that delta of 2 represents counter increased by 2, not incremented twice
- Made the explanation more precise about the wrap transitions

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
- Added build artifacts to .gitignore (Makefiles, config files, .deps directories)
- Removed accidentally committed build artifacts from repository

Co-authored-by: MothiShree <190452914+MothiShree@users.noreply.github.com>
Copilot AI changed the title [WIP] Apply changes based on feedback from review Refactor counter wrap-around logic into helper function Feb 17, 2026
Copilot AI requested a review from MothiShree February 17, 2026 10:44
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