8000 tests: remove test for strcasecmp by boretrk · Pull Request #6691 · libgit2/libgit2 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

tests: remove test for strcasecmp #6691

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

Merged
merged 2 commits into from
Dec 18, 2023
Merged

tests: remove test for strcasecmp #6691

merged 2 commits into from
Dec 18, 2023

Conversation

boretrk
Copy link
Contributor
@boretrk boretrk commented Dec 17, 2023

strcasecmp is a posix function, testing it doesn't make sense.
Functions that needs unsigned compare should use git__strcasecmp()

If people are using signed chars then they will probably run into some issue where sorted strings end up in a different order but this is a problem with the functions calling strcasecmp instead of git__strcasecmp, not strcasecmp itself.

strcasecmp is a posix function, testing it doesn't make sense.
Functions that needs unsigned compare should use git__strcasecmp()
It was unused and only defined in posix.h
It did not exist in its Windows equivalent.
@ethomson
Copy link
Member

This is one of those "what were we thinking!?" sort of moments. 🤔 And usually I can remember why we did something that seems silly five years down the line, but this one, I'm stumped.

@ethomson
Copy link
Member

I'm guessing that this was just a "let's show that our strcasecmp works like the standard" but... that's obviously not the case in a world where strcasecmp is driven by locale (and we're not explicitly setting it to C or something.)

So... anyway, this all seems good to me. 🙏

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

Successfully merging this pull request may close these issues.

2 participants
0