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.

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

refactor(cmdexpand): prefer memmove over strmove #29235

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

IAKOBVS
Copy link
Contributor

@IAKOBVS IAKOBVS commented Jun 8, 2024

When the length of the source string is known, use memmove over strmove to avoid the strlen.

@github-actions github-actions bot added the refactor changes that are not features or bugfixes label Jun 8, 2024
@clason
Copy link
Member

clason commented Jun 8, 2024

Again, why?

@lewis6991
Copy link
Member

It avoids the string being scanned for its length. A micro-op basically.

@IAKOBVS
Copy link
Contributor Author

IAKOBVS commented Jun 8, 2024

Again, why?

To avoid calling strlen for every STRMOVE in a loop since STRMOVE(dst, src) expands to memmove(dst, src, strlen(src) + 1) because here we know strlen(src) and that it is constant.

@clason
Copy link
Member

clason commented Jun 8, 2024

I'd be more excited about this if it prevented overflow and would shut up some code scanning tools.

@IAKOBVS
Copy link
Contributor Author

IAKOBVS commented Jun 8, 2024

Calling memmove in a loop is already quadratic. This changes it from 2 * n^2 operations to n^2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor changes that are not features or bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants