On Sun, Dec 15, 2013 at 3:32 PM, Nikita Karetnikov <nikita@karetnikov.org> wrote:
I’m trying to write a wrapper for a C function.  Here is an example of
such code:

...
  s' <- newCString s
  t' <- newCString t
  let n = c_strcmp s' t'
  -- free s'
  -- free t' 
...

It'd be better to use withCString [1] instead, to avoid a memory leak if an exception occurs between newCString and free.

Also, make c_strcmp an IO function:

    foreign import ccall "string.h strcmp"
        c_strcmp :: CString -> CString -> IO CInt

The way you had it, c_strcmp had an implicit unsafePerformIO, which we don't want or need here.  strcmp has the side effect of reading from pointers at a given moment in time.  As Brandon brought up, right now, your code might as well say:

    strcmp s t = unsafePerformIO $ do
      s' <- newCString s
      t' <- newCString t
      -- free s'
      -- free t'
      return $ case c_strcmp s' t' of
        _ | n == 0    -> EQ
          | n <  0    -> LT
          | otherwise -> GT

Because of laziness, n = c_strcmp s' t' isn't evaluated until it is needed.  What you should say instead is:

    ...
    n <- c_strcmp s' t'
    ...

This places the strcmp in the monadic chain so it will run at the right time (after newCString and before freeCString).


 [1]: http://hackage.haskell.org/package/base/docs/Foreign-C-String.html#v:withCString