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

Add peekCString, withCString for converting NUL terminated C strings #254

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

Conversation

andersk
Copy link

@andersk andersk commented Mar 13, 2019

Since NUL-terminated CString is much more common in foreign APIs than CStringLen, one should not have to manually build these functions out of peekCStringLen, withCStringLen (and perhaps get it wrong, like I did in jgm/cmark-hs#13).

While here, document that peekCStringLen, withCStringLen are O(n) as well.

Fixes #32.

Since NUL-terminated CString is much more common in foreign APIs than
CStringLen, one should not have to manually build these functions out
of `peekCStringLen`, `withCStringLen` (and perhaps get it wrong, like
I did in jgm/cmark-hs#13).

While here, document that `withCStringLen` is O(n) as well.

Fixes haskell#32.

Signed-off-by: Anders Kaseorg <[email protected]>
--
-- @since 1.2.5.0
withCString :: Text -> (CString -> IO a) -> IO a
withCString t act = useAsCString (encodeUtf8 t) act
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since text-2 this does redundant copying.

--
-- @since 1.2.5.0
withCString :: Text -> (CString -> IO a) -> IO a
withCString t act = useAsCString (encodeUtf8 t) act
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since text-2 this does redundant copying.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's redundant in both. encodeUtf8 creates a bytestring, which can be used unsafely, as nothing else refers to it.

In text-2.0 one could have unsafe withCString (or just withCStringLen, as there is no NUL at the end) variants though, which would just pass the Ptr to ByteArray contents.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrap that. text ByteArray's are not (always) pinned, so sometimes copying is unavoidable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least one round of memcpy is unavoidable unless the byte array in Text has that final null.

@dpwiz dpwiz mentioned this pull request May 11, 2022
@Bodigrim Bodigrim marked this pull request as draft February 28, 2023 18:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please provide peekUTF8CString and withUTF8CString
4 participants