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

Fix withFilePath #294

Merged
merged 2 commits into from
Sep 16, 2023
Merged

Fix withFilePath #294

merged 2 commits into from
Sep 16, 2023

Conversation

hasufell
Copy link
Member

haskell/cabal#9241

@Bodigrim we'll need another release. This bug was introduced by 2.8.2.0, so maybe we can mark it as deprecated on hackage?

@hasufell
Copy link
Member Author

@bgamari

@vdukhovni
Copy link

Sorry about that. I think the fault was mostly mine. Mind you the docs of useAsCStringLen could be more clear. They don't mention lack of NUL-termination, one has to carefully contrast with useAsCString to notice that only the latter mentions NUL-termination, and so implicitly perhaps the former does not NUL-terminate. I guess that choice is natural...

@hasufell
Copy link
Member Author

Well, I was well aware of this intricacy, because I worked long enough on the ShortByteString code. So I'm not sure how that slipped through my radar either.

@Bodigrim
Copy link
Contributor

Thanks for a quick turnaround!

I did not follow the discussion closely, is it possible to add a regression test?

In the meantime I've revised unix-2.8.2.0 with base < 0.

@hasufell
Copy link
Member Author

Yes, we could generate random filepaths, run withFilePath and just print the CString.

I don't know if I will have time to do that today.

@Bodigrim
Copy link
Contributor

A simple unit test will do IMHO, just to prevent someone in future to refactor back to useAsCStringLen.

@hasufell
Copy link
Member Author

A simple unit test will do IMHO, just to prevent someone in future to refactor back to useAsCStringLen.

I don't think so. This is a memory bug and doesn't always trigger.

@bgamari
Copy link
Contributor

bgamari commented Sep 13, 2023

Thanks for the heads up! Given the severity of the issue and the fact that it made it into a release, it would probably be a good idea to open a ticket and point to it from the changelog. I have done for former in #295; @hasufell, perhaps you can do the latter?

@erikd
Copy link
Member

erikd commented Sep 14, 2023

I just got hit by this. 🎉

Ran cabal update and now the bad version has been marked "unbuildable".

@Bodigrim Bodigrim mentioned this pull request Sep 15, 2023
@Bodigrim
Copy link
Contributor

@hasufell could you please rebase?

@hasufell
Copy link
Member Author

rebased

@hasufell
Copy link
Member Author

CI all green. Changelog updated. Version bumped to 2.8.2.1

@hasufell hasufell merged commit 3f0d217 into haskell:master Sep 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants