-
-
Notifications
You must be signed in to change notification settings - Fork 169
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
fix: Correctly create directories for paths with forward slashes on Windows #266
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! We should have a unit test for this.
a56cab6
to
678e4c3
Compare
@Swatinem updated |
Having this tested would be nice. I think its enough to just add a sentry-native/tests/unit/test_path.c Line 10 in 5020ff5
|
@Swatinem tests added. I do not known, if backslashes should be tested on non-windows platforms too? update: it works. |
66525f5
to
f3da259
Compare
Sorry for the back and forth here… Unices are apparently very forgiving when it comes to paths, but you do have an error in there.
|
f3da259
to
a87f4ef
Compare
fixed
My point that |
The reason I would like to compare to manually calling sentry-native/src/path/sentry_path_unix.c Line 322 in aca8890
I think unixes will just happily create a directory named |
Which reminds me… should we maybe normalize separators on unix as well? How common is it to use |
Thanks, now I understood it. Yeah, it is definitely broken, because
At least it's possible that someone actually uses the backslash as a folder name in unix --> it will break sentry-native for him.
It will be failed, because |
We should only execute tests with |
a87f4ef
to
c1f5869
Compare
Done |
Fixes sentry-native initialization on Windows when databasepath has forward slashes.