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

there is no need for both peekCText and peekCAText to exist #80

Open
chessai opened this issue Nov 7, 2018 · 2 comments
Open

there is no need for both peekCText and peekCAText to exist #80

chessai opened this issue Nov 7, 2018 · 2 comments

Comments

@chessai
Copy link
Contributor

chessai commented Nov 7, 2018

when i made my PR to change things from String to Text, i made an effort not to remove or add any functions, so as to minimise the chance of introducing a bug. however i noticed you had two variants of this peek function in use, but they both have the same type and implementation. One should be removed - i would prefer to remove 'peekCAText' and keep 'peekCText'.

@chessai
Copy link
Contributor Author

chessai commented Nov 7, 2018

oh, i'm at least wrong about their implementations being the same: peekCAString and peekCString differ in the following way:

-- | Marshal a NUL terminated C string into a Haskell string.
--
peekCAString    :: CString -> IO String
peekCAString cp = do
  l <- lengthArray0 nUL cp
  if l <= 0 then return "" else loop "" (l-1)
  where
    loop s i = do
        xval <- peekElemOff cp i
        let val = castCCharToChar xval
        val `seq` if i <= 0 then return (val:s) else loop (val:s) (i-1)

-- | Marshal a NUL terminated C string into a Haskell string.
--
peekCString    :: CString -> IO String
peekCString s = getForeignEncoding >>= flip GHC.peekCString s

@chessai
Copy link
Contributor Author

chessai commented Nov 7, 2018

if #79 is decided on being ok and merged, this should definitely happen, since these peekC(A)String functions won't matter anymore.

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

No branches or pull requests

1 participant