Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

chore: adopt better keycmp() implementation #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Integral-Tech
Copy link
Contributor

According to the source code of strcmp() in glibc, the return value is the difference of first different character in two strings.

For keycmp(), we can adopt an implementation akin to strcmp() in glibc.

@Integral-Tech Integral-Tech changed the title chore: use better keycmp() implementation chore: adopt better keycmp() implementation Jan 18, 2025
@eafer
Copy link
Member

eafer commented Jan 21, 2025

Have you tested this code? Because in C, substraction of unsigned types will wrap around and remain positive. The glibc version works because unsigned char gets promoted to int beforehand. Yes, this is all very messy.

@Integral-Tech
Copy link
Contributor Author

@eafer

      unsigned char c1 = *p1++;
      unsigned char c2 = *p2++;
      int diff = c1 - c2;

I don't think unsigned char gets promoted to int beforehand in glibc strcmp().

@eafer
Copy link
Member

eafer commented Jan 21, 2025

The promotion is implicit.

@Integral-Tech
Copy link
Contributor Author

Integral-Tech commented Jan 21, 2025

@eafer IMO this code is akin to strcmp() in glibc, as the return value is int.

static int omap_keycmp(struct apfs_omap_key *k1, struct apfs_omap_key *k2)
{
	if (le64_to_cpu(k1->ok_oid) != le64_to_cpu(k2->ok_oid))
		return le64_to_cpu(k1->ok_oid) - le64_to_cpu(k2->ok_oid);
	if (le64_to_cpu(k1->ok_xid) != le64_to_cpu(k2->ok_xid))
		return le64_to_cpu(k1->ok_xid) - le64_to_cpu(k2->ok_xid);
	return 0;
}

@eafer
Copy link
Member

eafer commented Jan 21, 2025

The cast only happens after all the unsigned math is done, so the result could wrap around so far that it becomes a positive int.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants