From 1904020d7f05b071ee26c0c282868252a74495fd Mon Sep 17 00:00:00 2001 From: akallabeth Date: Fri, 10 Sep 2021 09:06:35 +0200 Subject: [PATCH] Some code cleanups and WINPR_ASSERT (#7281) --- channels/rail/client/rail_main.c | 1 - client/Sample/tf_freerdp.c | 45 ++++++++++++++++++++++++++++-- include/freerdp/freerdp.h | 2 +- libfreerdp/cache/bitmap.c | 10 ++++++- libfreerdp/cache/brush.c | 12 +++++++- libfreerdp/cache/glyph.c | 5 ++++ libfreerdp/cache/pointer.c | 9 +++++- libfreerdp/core/connection.c | 2 +- libfreerdp/core/connection.h | 2 +- libfreerdp/core/freerdp.c | 2 +- libfreerdp/core/gateway/rpc_bind.c | 2 +- libfreerdp/core/peer.c | 41 +++++++++++---------------- libfreerdp/gdi/gdi.c | 13 ++++++++- libfreerdp/gdi/gfx.c | 19 ++++++++++--- 14 files changed, 125 insertions(+), 40 deletions(-) diff --git a/channels/rail/client/rail_main.c b/channels/rail/client/rail_main.c index 58a461cd3..64f0f416b 100644 --- a/channels/rail/client/rail_main.c +++ b/channels/rail/client/rail_main.c @@ -590,7 +590,6 @@ static UINT rail_virtual_channel_event_connected(railPlugin* rail, LPVOID pData, return rail->channelEntryPoints.pVirtualChannelOpenEx(rail->InitHandle, &rail->OpenHandle, rail->channelDef.name, rail_virtual_channel_open_event_ex); - ; } /** diff --git a/client/Sample/tf_freerdp.c b/client/Sample/tf_freerdp.c index e85f3bfd7..b264260f2 100644 --- a/client/Sample/tf_freerdp.c +++ b/client/Sample/tf_freerdp.c @@ -39,6 +39,7 @@ #include #include +#include #include #include @@ -51,7 +52,16 @@ * It can be used to reset invalidated areas. */ static BOOL tf_begin_paint(rdpContext* context) { - rdpGdi* gdi = context->gdi; + rdpGdi* gdi; + + WINPR_ASSERT(context); + + gdi = context->gdi; + WINPR_ASSERT(gdi); + WINPR_ASSERT(gdi->primary); + WINPR_ASSERT(gdi->primary->hdc); + WINPR_ASSERT(gdi->primary->hdc->hwnd); + WINPR_ASSERT(gdi->primary->hdc->hwnd->invalid); gdi->primary->hdc->hwnd->invalid->null = TRUE; return TRUE; } @@ -62,7 +72,16 @@ static BOOL tf_begin_paint(rdpContext* context) */ static BOOL tf_end_paint(rdpContext* context) { - rdpGdi* gdi = context->gdi; + rdpGdi* gdi; + + WINPR_ASSERT(context); + + gdi = context->gdi; + WINPR_ASSERT(gdi); + WINPR_ASSERT(gdi->primary); + WINPR_ASSERT(gdi->primary->hdc); + WINPR_ASSERT(gdi->primary->hdc->hwnd); + WINPR_ASSERT(gdi->primary->hdc->hwnd->invalid); if (gdi->primary->hdc->hwnd->invalid->null) return TRUE; @@ -70,6 +89,20 @@ static BOOL tf_end_paint(rdpContext* context) return TRUE; } +static BOOL tf_desktop_resize(rdpContext* context) +{ + rdpGdi* gdi; + rdpSettings* settings; + + WINPR_ASSERT(context); + + settings = context->settings; + WINPR_ASSERT(settings); + + gdi = context->gdi; + return gdi_resize(gdi, settings->DesktopWidth, settings->DesktopHeight); +} + /* This function is called to output a System BEEP */ static BOOL tf_play_sound(rdpContext* context, const PLAY_SOUND_UPDATE* play_sound) { @@ -107,7 +140,12 @@ static BOOL tf_keyboard_set_ime_status(rdpContext* context, UINT16 imeId, UINT32 static BOOL tf_pre_connect(freerdp* instance) { rdpSettings* settings; + + WINPR_ASSERT(instance); + settings = instance->settings; + WINPR_ASSERT(settings); + /* Optional OS identifier sent to server */ settings->OsMajorType = OSMAJORTYPE_UNIX; settings->OsMinorType = OSMINORTYPE_NATIVE_XSERVER; @@ -145,6 +183,7 @@ static BOOL tf_post_connect(freerdp* instance) instance->update->BeginPaint = tf_begin_paint; instance->update->EndPaint = tf_end_paint; instance->update->PlaySound = tf_play_sound; + instance->update->DesktopResize = tf_desktop_resize; instance->update->SetKeyboardIndicators = tf_keyboard_set_indicators; instance->update->SetKeyboardImeStatus = tf_keyboard_set_ime_status; return TRUE; @@ -311,6 +350,8 @@ static int tf_client_stop(rdpContext* context) static int RdpClientEntry(RDP_CLIENT_ENTRY_POINTS* pEntryPoints) { + WINPR_ASSERT(pEntryPoints); + ZeroMemory(pEntryPoints, sizeof(RDP_CLIENT_ENTRY_POINTS)); pEntryPoints->Version = RDP_CLIENT_INTERFACE_VERSION; pEntryPoints->Size = sizeof(RDP_CLIENT_ENTRY_POINTS_V1); diff --git a/include/freerdp/freerdp.h b/include/freerdp/freerdp.h index 8cd513e88..f911d3e2f 100644 --- a/include/freerdp/freerdp.h +++ b/include/freerdp/freerdp.h @@ -564,7 +564,7 @@ fingerprint. DEPRECATED: Use VerifyChangedCertificateEx */ FREERDP_API const char* freerdp_nego_get_routing_token(rdpContext* context, DWORD* length); - FREERDP_API CONNECTION_STATE freerdp_get_state(rdpContext* context); + FREERDP_API CONNECTION_STATE freerdp_get_state(const rdpContext* context); FREERDP_API const char* freerdp_state_string(CONNECTION_STATE state); #ifdef __cplusplus diff --git a/libfreerdp/cache/bitmap.c b/libfreerdp/cache/bitmap.c index 7baeec1e4..d00454dd7 100644 --- a/libfreerdp/cache/bitmap.c +++ b/libfreerdp/cache/bitmap.c @@ -261,7 +261,15 @@ BOOL bitmap_cache_put(rdpBitmapCache* bitmapCache, UINT32 id, UINT32 index, rdpB void bitmap_cache_register_callbacks(rdpUpdate* update) { - rdpCache* cache = update->context->cache; + rdpCache* cache; + + WINPR_ASSERT(update); + WINPR_ASSERT(update->context); + WINPR_ASSERT(update->context->cache); + + cache = update->context->cache; + WINPR_ASSERT(cache); + cache->bitmap->MemBlt = update->primary->MemBlt; cache->bitmap->Mem3Blt = update->primary->Mem3Blt; update->primary->MemBlt = update_gdi_memblt; diff --git a/libfreerdp/cache/brush.c b/libfreerdp/cache/brush.c index 207819a56..f41964ffb 100644 --- a/libfreerdp/cache/brush.c +++ b/libfreerdp/cache/brush.c @@ -229,7 +229,17 @@ void brush_cache_put(rdpBrushCache* brushCache, UINT32 index, void* entry, UINT3 void brush_cache_register_callbacks(rdpUpdate* update) { - rdpCache* cache = update->context->cache; + rdpCache* cache; + + WINPR_ASSERT(update); + WINPR_ASSERT(update->context); + WINPR_ASSERT(update->primary); + WINPR_ASSERT(update->secondary); + + cache = update->context->cache; + WINPR_ASSERT(cache); + WINPR_ASSERT(cache->brush); + cache->brush->PatBlt = update->primary->PatBlt; cache->brush->PolygonSC = update->primary->PolygonSC; cache->brush->PolygonCB = update->primary->PolygonCB; diff --git a/libfreerdp/cache/glyph.c b/libfreerdp/cache/glyph.c index ba2d921a0..03c750af9 100644 --- a/libfreerdp/cache/glyph.c +++ b/libfreerdp/cache/glyph.c @@ -663,6 +663,11 @@ BOOL glyph_cache_fragment_put(rdpGlyphCache* glyphCache, UINT32 index, UINT32 si void glyph_cache_register_callbacks(rdpUpdate* update) { + WINPR_ASSERT(update); + WINPR_ASSERT(update->context); + WINPR_ASSERT(update->primary); + WINPR_ASSERT(update->secondary); + update->primary->GlyphIndex = update_gdi_glyph_index; update->primary->FastIndex = update_gdi_fast_index; update->primary->FastGlyph = update_gdi_fast_glyph; diff --git a/libfreerdp/cache/pointer.c b/libfreerdp/cache/pointer.c index 31ba71352..9ceea0afa 100644 --- a/libfreerdp/cache/pointer.c +++ b/libfreerdp/cache/pointer.c @@ -306,7 +306,14 @@ BOOL pointer_cache_put(rdpPointerCache* pointer_cache, UINT32 index, rdpPointer* void pointer_cache_register_callbacks(rdpUpdate* update) { - rdpPointerUpdate* pointer = update->pointer; + rdpPointerUpdate* pointer; + + WINPR_ASSERT(update); + WINPR_ASSERT(update->context); + + pointer = update->pointer; + WINPR_ASSERT(pointer); + pointer->PointerPosition = update_pointer_position; pointer->PointerSystem = update_pointer_system; pointer->PointerColor = update_pointer_color; diff --git a/libfreerdp/core/connection.c b/libfreerdp/core/connection.c index 9e13a8fe3..feed2247e 100644 --- a/libfreerdp/core/connection.c +++ b/libfreerdp/core/connection.c @@ -1517,7 +1517,7 @@ const char* rdp_state_string(CONNECTION_STATE state) } } -CONNECTION_STATE rdp_get_state(rdpRdp* rdp) +CONNECTION_STATE rdp_get_state(const rdpRdp* rdp) { WINPR_ASSERT(rdp); return rdp->state; diff --git a/libfreerdp/core/connection.h b/libfreerdp/core/connection.h index 951e9fd8f..4be39ddfb 100644 --- a/libfreerdp/core/connection.h +++ b/libfreerdp/core/connection.h @@ -48,7 +48,7 @@ FREERDP_LOCAL int rdp_client_connect_license(rdpRdp* rdp, wStream* s); FREERDP_LOCAL int rdp_client_connect_demand_active(rdpRdp* rdp, wStream* s); FREERDP_LOCAL int rdp_client_transition_to_state(rdpRdp* rdp, CONNECTION_STATE state); -FREERDP_LOCAL CONNECTION_STATE rdp_get_state(rdpRdp* rdp); +FREERDP_LOCAL CONNECTION_STATE rdp_get_state(const rdpRdp* rdp); FREERDP_LOCAL const char* rdp_state_string(CONNECTION_STATE state); FREERDP_LOCAL BOOL rdp_server_accept_nego(rdpRdp* rdp, wStream* s); diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index 7d2efa717..54c2bbc8d 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -1008,7 +1008,7 @@ BOOL freerdp_set_io_callbacks(rdpContext* context, const rdpTransportIo* io_call return rdp_set_io_callbacks(context->rdp, io_callbacks); } -CONNECTION_STATE freerdp_get_state(rdpContext* context) +CONNECTION_STATE freerdp_get_state(const rdpContext* context) { WINPR_ASSERT(context); return rdp_get_state(context->rdp); diff --git a/libfreerdp/core/gateway/rpc_bind.c b/libfreerdp/core/gateway/rpc_bind.c index 0179b335e..2dad3206a 100644 --- a/libfreerdp/core/gateway/rpc_bind.c +++ b/libfreerdp/core/gateway/rpc_bind.c @@ -132,7 +132,7 @@ int rpc_send_bind_pdu(rdpRpc* rpc) WINPR_ASSERT(instance); connection = rpc->VirtualConnection; - ; + WINPR_ASSERT(connection); inChannel = connection->DefaultInChannel; diff --git a/libfreerdp/core/peer.c b/libfreerdp/core/peer.c index 9f816354e..99348ac12 100644 --- a/libfreerdp/core/peer.c +++ b/libfreerdp/core/peer.c @@ -910,7 +910,7 @@ BOOL freerdp_peer_context_new(freerdp_peer* client) return FALSE; if (!(context = (rdpContext*)calloc(1, client->ContextSize))) - goto fail_context; + goto fail; client->context = context; context->peer = client; @@ -918,10 +918,10 @@ BOOL freerdp_peer_context_new(freerdp_peer* client) context->settings = client->settings; if (!(context->metrics = metrics_new(context))) - goto fail_metrics; + goto fail; if (!(rdp = rdp_new(context))) - goto fail_rdp; + goto fail; client->update = rdp->update; client->settings = rdp->settings; @@ -940,11 +940,11 @@ BOOL freerdp_peer_context_new(freerdp_peer* client) if (!(context->errorDescription = calloc(1, 500))) { WLog_ERR(TAG, "calloc failed!"); - goto fail_error_description; + goto fail; } if (!transport_attach(rdp->transport, client->sockfd)) - goto fail_transport_attach; + goto fail; transport_set_recv_callbacks(rdp->transport, peer_recv_callback, client); transport_set_blocking_mode(rdp->transport, FALSE); @@ -957,18 +957,9 @@ BOOL freerdp_peer_context_new(freerdp_peer* client) if (ret) return TRUE; +fail: WLog_ERR(TAG, "ContextNew callback failed"); -fail_transport_attach: - free(context->errorDescription); -fail_error_description: - rdp_free(client->context->rdp); -fail_rdp: - metrics_free(context->metrics); -fail_metrics: - free(client->context); -fail_context: - client->context = NULL; - WLog_ERR(TAG, "Failed to create new peer context"); + freerdp_peer_context_free(client); return FALSE; } @@ -981,14 +972,16 @@ void freerdp_peer_context_free(freerdp_peer* client) if (client->context) { - free(client->context->errorDescription); - client->context->errorDescription = NULL; - rdp_free(client->context->rdp); - client->context->rdp = NULL; - metrics_free(client->context->metrics); - client->context->metrics = NULL; - free(client->context); - client->context = NULL; + rdpContext* ctx = client->context; + + free(ctx->errorDescription); + ctx->errorDescription = NULL; + rdp_free(ctx->rdp); + ctx->rdp = NULL; + metrics_free(ctx->metrics); + ctx->metrics = NULL; + free(ctx); + ctx = NULL; } } diff --git a/libfreerdp/gdi/gdi.c b/libfreerdp/gdi/gdi.c index b1ef93c54..bc9cba1ac 100644 --- a/libfreerdp/gdi/gdi.c +++ b/libfreerdp/gdi/gdi.c @@ -1112,7 +1112,18 @@ out: static void gdi_register_update_callbacks(rdpUpdate* update) { - rdpPrimaryUpdate* primary = update->primary; + rdpPrimaryUpdate* primary; + const rdpSettings* settings; + + WINPR_ASSERT(update); + WINPR_ASSERT(update->context); + + settings = update->context->settings; + WINPR_ASSERT(settings); + + primary = update->primary; + WINPR_ASSERT(primary); + update->Palette = gdi_palette_update; update->SetBounds = gdi_set_bounds; primary->DstBlt = gdi_dstblt; diff --git a/libfreerdp/gdi/gfx.c b/libfreerdp/gdi/gfx.c index a46716896..afe4661c8 100644 --- a/libfreerdp/gdi/gfx.c +++ b/libfreerdp/gdi/gfx.c @@ -102,7 +102,10 @@ static UINT gdi_ResetGraphics(RdpgfxClientContext* context, settings->DesktopHeight = DesktopHeight; if (update) + { + WINPR_ASSERT(update->DesktopResize); update->DesktopResize(gdi->context); + } context->GetSurfaceIds(context, &pSurfaceIds, &count); @@ -258,8 +261,14 @@ static UINT gdi_StartFrame(RdpgfxClientContext* context, const RDPGFX_START_FRAM */ static UINT gdi_EndFrame(RdpgfxClientContext* context, const RDPGFX_END_FRAME_PDU* endFrame) { - UINT status = CHANNEL_RC_NOT_INITIALIZED; - rdpGdi* gdi = (rdpGdi*)context->custom; + UINT status = CHANNEL_RC_OK; + rdpGdi* gdi; + + WINPR_ASSERT(context); + WINPR_ASSERT(endFrame); + + gdi = (rdpGdi*)context->custom; + WINPR_ASSERT(gdi); IFCALLRET(context->UpdateSurfaces, status, context); gdi->inGfxFrame = FALSE; return status; @@ -1514,11 +1523,13 @@ BOOL gdi_graphics_pipeline_init_ex(rdpGdi* gdi, RdpgfxClientContext* gfx, pcRdpgfxUpdateSurfaceArea update) { rdpContext* context; + const rdpSettings* settings; if (!gdi || !gfx || !gdi->context || !gdi->context->settings) return FALSE; context = gdi->context; + settings = gdi->context->settings; gdi->gfx = gfx; gfx->custom = (void*)gdi; @@ -1547,8 +1558,8 @@ BOOL gdi_graphics_pipeline_init_ex(rdpGdi* gdi, RdpgfxClientContext* gfx, gfx->codecs = codecs_new(context); if (!gfx->codecs) return FALSE; - freerdp_client_codecs_prepare(gfx->codecs, FREERDP_CODEC_ALL, context->settings->DesktopWidth, - context->settings->DesktopHeight); + freerdp_client_codecs_prepare(gfx->codecs, FREERDP_CODEC_ALL, settings->DesktopWidth, + settings->DesktopHeight); InitializeCriticalSection(&gfx->mux); PROFILER_CREATE(gfx->SurfaceProfiler, "GFX-PROFILER")