5
\$\begingroup\$

This is a function definition from an external dll, that I'm calling from my code:

LZHAM_DLL_EXPORT lzham_compress_status_t LZHAM_CDECL lzham_compress(
  lzham_compress_state_ptr pState,
  const lzham_uint8 *pIn_buf, size_t *pIn_buf_size,
  lzham_uint8 *pOut_buf, size_t *pOut_buf_size,
  lzham_bool no_more_input_bytes_flag);

This function gets an opaque pointer pState which is returned from another api call, buffer and size of input block to compress, and buffer and size to write the output to. As the comment have it (reformatted):

// Compresses an arbitrarily sized block of data, writing as much available 
// compressed data as possible to the output buffer. 
// This method may be called as many times as needed, but for best perf. try 
// not to call it with tiny buffers.
// pState - Pointer to internal compression state, created by 
// lzham_compress_init.
// pIn_buf, pIn_buf_size - Pointer to input data buffer, and pointer to a 
// size_t containing the number of bytes available in this buffer. 
// On return, *pIn_buf_size will be set to the number of bytes read from the buffer.
// pOut_buf, pOut_buf_size - Pointer to the output data buffer, and a 
// pointer to a size_t containing the max number of bytes that can be 
// written to this buffer.
// On return, *pOut_buf_size will be set to the number of bytes written to this buffer.
// no_more_input_bytes_flag - Set to true to indicate that no more input 
// bytes are available to compress (EOF). Once you call this function with 
// this param set to true, it must stay set to true in all future calls.

I would like to know if my code below is overly verbose, and if it can be improved by specifying MarshalAs attributes or in any other way. I'm suspecting that It can be written more idiomatically than my attempt.

[DllImport(LzhamDll, CallingConvention = CallingConvention.Cdecl)]
private static extern unsafe int lzham_compress(IntPtr state, byte* inBuf,
    ref IntPtr inBufSize, byte* outBuf,
    ref IntPtr outBufSize, int noMoreInputBytesFlag);

public static unsafe CompressStatus Compress(IntPtr state, byte[] inBuf, ref 
    int inBufSize, byte[] outBuf, ref int outBufSize, int noMoreInputBytesFlag)
{
    fixed (byte* inBytes = inBuf)
    fixed (byte* outBytes = outBuf)
    {
        IntPtr inSize = new IntPtr(inBufSize);
        IntPtr outSize = new IntPtr(outBufSize);
        CompressStatus result = (CompressStatus)lzham_compress(state, 
            inBytes, ref inSize, outBytes, ref outSize, noMoreInputBytesFlag);
        inBufSize = inBufSize - inSize.ToInt32();
        outBufSize = outBufSize - outSize.ToInt32();
        return result;
    }
}

Above, CompressStatus is a simple enum with all possible return status codes.

\$\endgroup\$
  • \$\begingroup\$ You might want to turn state into a SafeHandle. \$\endgroup\$ – CodesInChaos Oct 16 '15 at 12:58
1
\$\begingroup\$

This declaration uses CLR arrays as parameters and does not have unsafe values:

[DllImport(LzhamDll, CallingConvention = CallingConvention.Cdecl)]
[return: MarshalAs(UnmanagedType.I4)]
private static extern CompressStatus lzham_compress(
    IntPtr state,
    [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 2)]byte[] inBuf,
    ref uint inBufSize,
    [MarshalAs(UnmanagedType.LPArray, SizeParamIndex = 4)]byte[] outBuf,
    ref uint outBufSize,
    int noMoreInputBytesFlag);
\$\endgroup\$
  • 4
    \$\begingroup\$ Hello, please explain why this is better than the OP's code. It'll make for a better review! \$\endgroup\$ – IEatBagels Oct 29 '15 at 13:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.