[PATCH] ole32: Add user marshalling for bitmaps.

Zebediah Figura zfigura at codeweavers.com
Fri Dec 1 10:08:53 CST 2017


On 12/01/2017 04:21 AM, Huw Davies wrote:
> On Thu, Nov 30, 2017 at 07:14:56PM -0600, Zebediah Figura wrote:
>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>> ---
>>  dlls/ole32/tests/usrmarshal.c |  46 +++++++++++++++
>>  dlls/ole32/usrmarshal.c       | 126 +++++++++++++++++++++++++++++++++++++-----
>>  2 files changed, 158 insertions(+), 14 deletions(-)
>>
>> diff --git a/dlls/ole32/tests/usrmarshal.c b/dlls/ole32/tests/usrmarshal.c
>> index 529ad757c6a..ba62bb08c23 100644
>> --- a/dlls/ole32/tests/usrmarshal.c
>> +++ b/dlls/ole32/tests/usrmarshal.c
>> @@ -1190,6 +1190,51 @@ static void test_marshal_HBRUSH(void)
>>      DeleteObject(hBrush);
>>  }
>>  
>> +static void test_marshal_HBITMAP(void)
>> +{
>> +    static const ULONG header_size = FIELD_OFFSET(userBITMAP, cbSize);
>> +    static BYTE bmp_bits[1024];
>> +    MIDL_STUB_MESSAGE stub_msg;
>> +    HBITMAP hBitmap, hBitmap2;
>> +    USER_MARSHAL_CB umcb;
>> +    RPC_MESSAGE rpc_msg;
>> +    unsigned char *buffer;
>> +    unsigned char bitmap[1024];
>> +    ULONG size, bitmap_size;
>> +
>> +    memset(bmp_bits, 0xbb, sizeof(bmp_bits));
>> +    hBitmap = CreateBitmap(16, 16, 1, 1, bmp_bits);
>> +    ok(hBitmap != 0, "CreateBitmap failed\n");
>> +    size = GetObjectA(hBitmap, sizeof(bitmap), bitmap);
>> +    ok(size != 0, "GetObject failed\n");
>> +    bitmap_size = GetBitmapBits(hBitmap, 0, NULL);
>> +    ok(bitmap_size != 0, "GetBitmapBits failed\n");
>> +
>> +    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
>> +    size = HBITMAP_UserSize(&umcb.Flags, 0, &hBitmap);
> 
> Pass in a 1 rather than 0 to test alignment.
> 
>> +    ok(size == 0xc + header_size + bitmap_size ||
>> +       broken(size == 0x10 + header_size + bitmap_size), /* Windows adds 4 extra (unused) bytes */
>> +       "Wrong size %d\n", size);
>> +
>> +    buffer = HeapAlloc(GetProcessHeap(), 0, size);
>> +    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
>> +    HBITMAP_UserMarshal(&umcb.Flags, buffer, &hBitmap);
> 
> Similarly buffer + 1
> 
> Could you also test the return value.
> 
>> +    ok(*(ULONG *)buffer == WDT_REMOTE_CALL, "Context should be WDT_REMOTE_CALL instead of 0x%08x\n", *(ULONG *)buffer);
>> +    ok(*(ULONG *)(buffer + 0x4) == (ULONG)(ULONG_PTR)hBitmap, "wirestgm + 0x4 should be bitmap handle instead of 0x%08x\n", *(ULONG *)(buffer + 0x4));
>> +    ok(*(ULONG *)(buffer + 0x8) == (ULONG)(ULONG_PTR)bitmap_size, "wirestgm + 0x8 should be bitmap size instead of 0x%08x\n", *(ULONG *)(buffer + 0x4));
>> +    ok(!memcmp(buffer + 0xc, bitmap, header_size), "buffer mismatch\n");
>> +    ok(!memcmp(buffer + 0xc + header_size, bmp_bits, bitmap_size), "buffer mismatch\n");
>> +
>> +    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, buffer, size, MSHCTX_LOCAL);
>> +    HBITMAP_UserUnmarshal(&umcb.Flags, buffer, &hBitmap2);
>> +    ok(hBitmap2 != NULL, "Didn't unmarshal properly\n");
>> +    HeapFree(GetProcessHeap(), 0, buffer);
>> +
>> +    init_user_marshal_cb(&umcb, &stub_msg, &rpc_msg, NULL, 0, MSHCTX_LOCAL);
>> +    HBITMAP_UserFree(&umcb.Flags, &hBitmap2);
>> +    DeleteObject(hBitmap);
>> +}
> 
> It would be nice to have the MSHCTX_INPROC stuff tested too, since
> you're implementing that.
> 
>> diff --git a/dlls/ole32/usrmarshal.c b/dlls/ole32/usrmarshal.c
>> index 03ad689f03b..605cdbc1822 100644
>> --- a/dlls/ole32/usrmarshal.c
>> +++ b/dlls/ole32/usrmarshal.c
>> @@ -586,8 +586,26 @@ void __RPC_USER HGLOBAL_UserFree(ULONG *pFlags, HGLOBAL *phGlobal)
>>   */
>>  ULONG __RPC_USER HBITMAP_UserSize(ULONG *pFlags, ULONG StartingSize, HBITMAP *phBmp)
>>  {
>> -    FIXME(":stub\n");
>> -    return StartingSize;
>> +    ULONG size = StartingSize;
> 
> Rename the parameter StartingSize to size and you're done.  In fact
> let's take the opportunity to rename all of the parameters to flags,
> size and bmp.  Do the same with the others below.
> 
>> +
>> +    TRACE("(%s, %d, %p)\n", debugstr_user_flags(pFlags), StartingSize, *phBmp);
> 
> ALIGN_LENGTH(size, 3);
> 
>> +
>> +    size += sizeof(ULONG);
>> +    if (LOWORD(*pFlags) == MSHCTX_INPROC)
>> +        size += sizeof(ULONG_PTR);
>> +    else
>> +    {
>> +        size += sizeof(ULONG);
>> +
>> +        if (*phBmp)
>> +        {
>> +            size += sizeof(ULONG);
>> +            size += FIELD_OFFSET(userBITMAP, cbSize);
>> +            size += GetBitmapBits(*phBmp, 0, NULL);
>> +        }
>> +    }
>> +
>> +    return size;
>>  }
>>  
>>  /******************************************************************************
>> @@ -611,7 +629,43 @@ ULONG __RPC_USER HBITMAP_UserSize(ULONG *pFlags, ULONG StartingSize, HBITMAP *ph
>>  */
>>  unsigned char * __RPC_USER HBITMAP_UserMarshal(ULONG *pFlags, unsigned char *pBuffer, HBITMAP *phBmp)
>>  {
>> -    FIXME(":stub\n");
>> +    TRACE("(%s, %p, %p)\n", debugstr_user_flags(pFlags), pBuffer, *phBmp);
> 
> ALIGN_POINTER(buffer, 3);
> 
>> +
>> +    if (LOWORD(*pFlags) == MSHCTX_INPROC)
>> +    {
>> +        if (sizeof(*phBmp) == 8)
>> +            *(ULONG *)pBuffer = WDT_INPROC64_CALL;
>> +        else
>> +            *(ULONG *)pBuffer = WDT_INPROC_CALL;
>> +        pBuffer += sizeof(ULONG);
>> +        *(HBITMAP *)pBuffer = *phBmp;
>> +        pBuffer += sizeof(HBITMAP);
>> +    }
>> +    else
>> +    {
>> +        *(ULONG *)pBuffer = WDT_REMOTE_CALL;
>> +        pBuffer += sizeof(ULONG);
>> +        *(ULONG *)pBuffer = (ULONG)(ULONG_PTR)*phBmp;
>> +        pBuffer += sizeof(ULONG);
>> +
>> +        if (*phBmp)
>> +        {
>> +            static const ULONG header_size = FIELD_OFFSET(userBITMAP, cbSize);
>> +            BITMAP bitmap;
>> +            ULONG bitmap_size;
>> +
>> +            bitmap_size = GetBitmapBits(*phBmp, 0, NULL);
>> +            *(ULONG *)pBuffer = bitmap_size;
>> +            pBuffer += sizeof(ULONG);
>> +
>> +            GetObjectW(*phBmp, sizeof(BITMAP), &bitmap);
>> +            memcpy(pBuffer, &bitmap, header_size);
>> +            pBuffer += header_size;
>> +
>> +            GetBitmapBits(*phBmp, bitmap_size, pBuffer);
>> +            pBuffer += bitmap_size;
>> +        }
>> +    }
>>      return pBuffer;
>>  }
>>  
>> @@ -636,7 +690,54 @@ unsigned char * __RPC_USER HBITMAP_UserMarshal(ULONG *pFlags, unsigned char *pBu
>>   */
>>  unsigned char * __RPC_USER HBITMAP_UserUnmarshal(ULONG *pFlags, unsigned char *pBuffer, HBITMAP *phBmp)
>>  {
>> -    FIXME(":stub\n");
>> +    ULONG fContext;
> 
> ULONG context;
> 
>> +
>> +    TRACE("(%s, %p, %p)\n", debugstr_user_flags(pFlags), pBuffer, phBmp);
>> +
>> +    fContext = *(ULONG *)pBuffer;
>> +    pBuffer += sizeof(ULONG);
>> +
>> +    if (((fContext == WDT_INPROC_CALL) && (sizeof(*phBmp) < 8)) ||
>> +        ((fContext == WDT_INPROC64_CALL) && (sizeof(*phBmp) == 8)))
>> +    {
>> +        *phBmp = *(HBITMAP *)pBuffer;
>> +        pBuffer += sizeof(*phBmp);
>> +    }
>> +    else if (fContext == WDT_REMOTE_CALL)
>> +    {
>> +        ULONG handle;
>> +
>> +        handle = *(ULONG *)pBuffer;
>> +        pBuffer += sizeof(ULONG);
>> +
>> +        if (handle)
>> +        {
>> +            static const ULONG header_size = FIELD_OFFSET(userBITMAP, cbSize);
>> +            BITMAP bitmap;
>> +            ULONG bitmap_size;
>> +            unsigned char *bits;
>> +
>> +            bitmap_size = *(ULONG *)pBuffer;
>> +            pBuffer += sizeof(ULONG);
>> +            bits = HeapAlloc(GetProcessHeap(), 0, bitmap_size);
>> +
>> +            memcpy(&bitmap, pBuffer, header_size);
>> +            pBuffer += header_size;
>> +
>> +            memcpy(bits, pBuffer, bitmap_size);
>> +            pBuffer += bitmap_size;
>> +
>> +            bitmap.bmBits = bits;
>> +            *phBmp = CreateBitmapIndirect(&bitmap);
>> +
>> +            HeapFree(GetProcessHeap(), 0, bits);
>> +        }
>> +        else
>> +            *phBmp = NULL;
>> +    }
>> +    else
>> +        RaiseException(RPC_S_INVALID_TAG, 0, 0, NULL);
>> +
>>      return pBuffer;
>>  }
>>  
>> @@ -660,7 +761,10 @@ unsigned char * __RPC_USER HBITMAP_UserUnmarshal(ULONG *pFlags, unsigned char *p
>>   */
>>  void __RPC_USER HBITMAP_UserFree(ULONG *pFlags, HBITMAP *phBmp)
>>  {
>> -    FIXME(":stub\n");
>> +    TRACE("(%s, %p)\n", debugstr_user_flags(pFlags), *phBmp);
>> +
>> +    if (LOWORD(*pFlags) != MSHCTX_INPROC)
>> +        DeleteObject(*phBmp);
>>  }
>>  
>>  /******************************************************************************
>> @@ -1620,9 +1724,7 @@ ULONG __RPC_USER STGMEDIUM_UserSize(ULONG *pFlags, ULONG StartingSize, STGMEDIUM
>>      case TYMED_GDI:
>>          TRACE("TYMED_GDI\n");
>>          if (pStgMedium->u.hBitmap)
>> -        {
>> -            FIXME("not implemented for GDI object %p\n", pStgMedium->u.hBitmap);
>> -        }
>> +            size = HBITMAP_UserSize(pFlags, size, &pStgMedium->u.hBitmap);
> 
> The stgmedium changes should probably be a separate patch.
> 
> Huw.
> 

Thanks for the review. I'll send an updated patchset.



More information about the wine-devel mailing list