Make some FFI argument passing changes/doc changes

Author: harendra-kumar

core/src/Streamly/Internal/Data/MutArray/Type.hs

@@ -438,10 +438,10 @@ import Prelude hiding
 -------------------------------------------------------------------------------
 
 -- NOTE: Have to be "ccall unsafe" so that we can pass unpinned memory to these
--- NOTE: cannot pass Ptr, have to pass MutableByteArray# for safe unpinned
--- memory access in C code.
+-- NOTE: for passing unpinned memory safely we have to pass unlifted pointers
+-- in FFI so that GC cannot change the wrapper during the call setup.
 foreign import ccall unsafe "string.h memcpy" c_memcpy
-    :: MutableByteArray# RealWorld -> Ptr Word8 -> CSize -> IO (Ptr Word8)
+    :: MutableByteArray# RealWorld -> Addr# -> CSize -> IO (Ptr Word8)
 
 -- XXX We can pass an Addr# to memchr instead of writing a C wrapper.
 foreign import ccall unsafe "memchr_index" c_memchr_index
@@ -2592,13 +2592,13 @@ fromPureStream xs =
 --
 -- /Unsafe:/
 --
--- 1. The memory pointed by @addr@ must be pinned or static.
+-- 1. The caller has to make sure the pointer is live during the call.
 -- 2. The caller is responsible to ensure that the pointer passed is valid up
 -- to the given length.
 --
 {-# INLINABLE fromPtrN #-}
 fromPtrN :: MonadIO m => Int -> Ptr Word8 -> m (MutArray Word8)
-fromPtrN len addr = do
+fromPtrN len (Ptr addr) = do
     -- memcpy is better than stream copy when the size is known.
     -- XXX We can implement a stream copy in a similar way by streaming Word64
     -- first and then remaining Word8.
@@ -2607,12 +2607,12 @@ fromPtrN len addr = do
     _ <- liftIO $ c_memcpy mbarr addr (fromIntegral len)
     pure (arr { arrEnd = len })
 
--- | @fromW16CString# addr@ copies a C string consisting of bytes and
+-- | @fromCString# addr@ copies a C string consisting of bytes and
 -- terminated by a null byte, into a Word8 array. The null byte is not copied.
 --
 -- /Unsafe:/
 --
--- 1. The memory pointed by @addr@ must be pinned or static.
+-- 1. The caller has to make sure that the @addr@ is alive during the call.
 -- 2. The caller is responsible to ensure that the pointer passed is valid up
 -- to the point where null byte is found.
 --
@@ -2641,7 +2641,7 @@ fromByteStr# = fromCString#
 --
 -- /Unsafe:/
 --
--- 1. The memory pointed by @addr@ must be pinned or static.
+-- 1. The caller has to make sure that the @addr@ is alive during the call.
 -- 2. The caller is responsible to ensure that the pointer passed is valid up
 -- to the point where null Word16 is found.
 --
@@ -2867,16 +2867,18 @@ unsafeSplice dst src = do
          (arrContents src) startSrc (arrContents dst) endDst srcLen
      return $ dst {arrEnd = endDst + srcLen}
 
--- |
+-- | Append specified number of bytes from a given pointer to the MutArray.
+--
 -- /Unsafe:/
 --
 -- 1. Does not check the length of the MutArray.
--- 2. The caller is responsible to ensure that the pointer passed is valid up
+-- 2. The caller has to make sure that the pointer is alive during the call
+-- 3. The caller is responsible to ensure that the pointer passed is valid up
 -- to the given length.
 --
 {-# INLINE unsafeAppendPtrN #-}
 unsafeAppendPtrN :: MonadIO m =>
-    MutArray a -> Ptr a -> Int -> m (MutArray a)
+    MutArray Word8 -> Ptr Word8 -> Int -> m (MutArray Word8)
 unsafeAppendPtrN dst ptr ptrLen = do
     let newEnd = arrEnd dst + ptrLen
     assertM(newEnd <= arrBound dst)
@@ -3094,6 +3096,7 @@ breakOn sep arr@MutArray{..} = liftIO $ do
     -- Need efficient stream based primitives that work on Word64.
     let marr = getMutByteArray# arrContents
         len = fromIntegral (arrEnd - arrStart)
+    -- XXX pass Addr# to memchr instead of using a C wrapper over memchr
     sepIndex <- c_memchr_index marr (fromIntegral arrStart) sep len
     let intIndex = fromIntegral sepIndex
     return $

core/src/Streamly/Internal/Data/MutByteArray/Type.hs

@@ -68,7 +68,6 @@ import Data.Word (Word8)
 import Debug.Trace (trace)
 #endif
 import Foreign.C.Types (CSize(..))
-import Foreign.Ptr (castPtr)
 import GHC.Base (IO(..))
 import System.IO.Unsafe (unsafePerformIO)
 
@@ -287,23 +286,26 @@ unsafePutSlice src srcStartBytes dst dstStartBytes lenBytes = liftIO $ do
                 , () #)
 
 foreign import ccall unsafe "string.h memcpy" c_memcpy
-    :: Addr# -> Ptr Word8 -> CSize -> IO (Ptr Word8)
+    :: Addr# -> Addr# -> CSize -> IO (Ptr Word8)
 
 -- | @unsafePutPtrN srcPtr dst dstOffset len@ copies @len@ bytes from @srcPtr@
 -- to dst at @dstOffset@.
 --
--- This is unsafe as it does not check the bounds of @dst@ and the user is
--- responsible to pass a valid pointer up to length @len@..
+-- /Unsafe/:
+--
+-- * does not check the bounds of the @dst@ array.
+-- * the caller must ensure that @srcPtr@ is alive during the call
+-- * the caller must ensure that @srcPtr@ is valid up to length @len@.
 --
 {-# INLINE unsafePutPtrN #-}
 unsafePutPtrN ::
        MonadIO m
-    => Ptr a
+    => Ptr Word8
     -> MutByteArray
     -> Int
     -> Int
     -> m ()
-unsafePutPtrN srcPtr dst dstOffset len = liftIO $ do
+unsafePutPtrN (Ptr srcAddr) dst dstOffset len = liftIO $ do
 #ifdef DEBUG
     dstLen <- length dst
     when (dstLen - dstOffset < len)
@@ -313,7 +315,7 @@ unsafePutPtrN srcPtr dst dstOffset len = liftIO $ do
     let !dstAddr# = (byteArrayContents# (unsafeCoerce# (getMutByteArray# dst)))
         !(I# dstOff#) = dstOffset
         !dstAddr1# = plusAddr# dstAddr# dstOff#
-    _ <- c_memcpy dstAddr1# (castPtr srcPtr) (fromIntegral len)
+    _ <- c_memcpy dstAddr1# srcAddr (fromIntegral len)
     pure ()
 
 -- | Unsafe as it does not check whether the start offset and length supplied

core/src/Streamly/Internal/FileSystem/Path/Common.hs

@@ -1809,6 +1809,8 @@ mkQ f =
 foreign import ccall unsafe "string.h strlen" c_strlen
     :: Addr# -> IO CSize
 
+-- | Append a separator and a CString to the Array.
+--
 {-# INLINE appendCString #-}
 appendCString :: OS -> Array Word8 -> CString -> IO (Array Word8)
 appendCString os a b@(Ptr addrB#) = do

core/src/Streamly/Internal/FileSystem/Posix/ReadDir.hsc

@@ -128,7 +128,6 @@ isMetaDir :: Ptr CChar -> IO Bool
 isMetaDir dname = do
     -- XXX Assuming an encoding that maps "." to ".", this is true for
     -- UTF8.
-    -- Load as soon as possible to optimize memory accesses
     c1 <- peek dname
     c2 :: Word8 <- peekByteOff dname 1
     if (c1 /= fromIntegral (ord '.'))
@@ -152,6 +151,8 @@ lstatDname parent dname = do
     if isMeta
     then pure (True, True)
     else do
+        -- XXX We can create a pinned array right here since the next call pins
+        -- it anyway.
         path <- appendCString parent dname
         Array.asCStringUnsafe (Path.toChunk path) $ \cStr -> do
             res <- c_lstat_is_directory cStr
@@ -285,8 +286,6 @@ eitherReader =
 {-# INLINE appendCString #-}
 appendCString :: PosixPath -> CString -> IO PosixPath
 appendCString x@(PosixPath a) b = do
-    -- XXX Should we ensure that this is pinned in the DT_UNKNOWN case
-    -- because we always pass it to a C function which pins it anyway.
     arr <- PathC.appendCString PathC.Posix a b
     pure $ PosixPath arr