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.

Sign up for GitHub

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

Jump to bottom

cmp: avoid repeated substr() on binary buffer #388

Merged
merged 2 commits into from
Dec 26, 2023
Merged

cmp: avoid repeated substr() on binary buffer #388

merged 2 commits into from
Dec 26, 2023

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Dec 25, 2023

  • Slightly simplify code by unpack()ing buffers that are not equal
  • $lines_read becomes a tally, and $report_lines is always ahead by one because $lines_read counts from zero
  • When printing the differences with -l flag, ord+substr pattern is avoided by using unpacked buffers
  • Retire EX_USAGE which had same value as EX_FAILURE

@mknos
cmp: avoid repeated substr() on binary buffer
7a22ea8 
* Slightly simplify code by unpack()ing buffers that are not equal
* $lines_read becomes a tally, and $report_lines is always ahead by one because $lines_read counts from zero
* When printing the differences with -l flag, ord+substr pattern is avoided by using unpacked buffers
* Retire EX_USAGE which had same value as EX_FAILURE
@github-actions github-actions bot added Type: enhancement improve a feature that already exists Priority: low get to this whenever Program: cmp The cop program labels Dec 25, 2023
@briandfoy
Copy link
Owner

Let's keep EX_USAGE. It doesn't matter that it has the same value as something else. It's possible to have a different value, is it's existence is a logical win. I'll fix up the patch because the rest is good.

@briandfoy briandfoy self-assigned this Dec 26, 2023
@briandfoy briandfoy merged commit 6546144 into briandfoy:master Dec 26, 2023
1 check passed
@briandfoy briandfoy added Status: accepted The fix is accepted and removed Priority: low get to this whenever labels Dec 26, 2023
@briandfoy briandfoy added Status: released there is a new release with this fix and removed Status: accepted The fix is accepted labels Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@briandfoy briandfoy briandfoy approved these changes

Assignees

@briandfoy briandfoy

Labels
Program: cmp The cop program Status: released there is a new release with this fix Type: enhancement improve a feature that already exists
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mknos @briandfoy