From 1c577e2a12387176e3870d7c5af282f7e6954ced Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Wed, 24 Aug 2011 09:27:56 +0200 Subject: [PATCH 1/3] Changed return value check for XGetWindowProperty XGetWindowProperty return None if the property isn't found. So raise an error is None is returned. Otherwise the property was returned. --- client/X11/xf_window.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/X11/xf_window.c b/client/X11/xf_window.c index 9a33e534d..c99657e63 100644 --- a/client/X11/xf_window.c +++ b/client/X11/xf_window.c @@ -94,7 +94,7 @@ boolean xf_GetWindowProperty(xfInfo* xfi, Window window, Atom property, int leng property, 0, length, False, AnyPropertyType, &actual_type, &actual_format, nitems, bytes, prop); - if (status != Success) + if (status == None) return False; return True; From f7336cea5eeb7c47d87df538a43342926bcc80f8 Mon Sep 17 00:00:00 2001 From: Bernhard Miklautz Date: Wed, 24 Aug 2011 09:12:07 +0200 Subject: [PATCH 2/3] Changed return values of freerdp_parse_args Return -1 if a argument is missing/wrong. This way caller can check for a problem. --- client/X11/xfreerdp.c | 4 +++- libfreerdp-utils/args.c | 38 +++++++++++++++++++------------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/client/X11/xfreerdp.c b/client/X11/xfreerdp.c index 4baf76ee9..2ca8aacd4 100644 --- a/client/X11/xfreerdp.c +++ b/client/X11/xfreerdp.c @@ -643,7 +643,9 @@ int main(int argc, char* argv[]) chanman = freerdp_chanman_new(); SET_CHANMAN(instance, chanman); - freerdp_parse_args(instance->settings, argc, argv, xf_process_plugin_args, chanman, NULL, NULL); + if (freerdp_parse_args(instance->settings, argc, argv, xf_process_plugin_args, chanman, NULL, + NULL) < 0) + return 1; data = (struct thread_data*) xzalloc(sizeof(struct thread_data)); data->instance = instance; diff --git a/libfreerdp-utils/args.c b/libfreerdp-utils/args.c index 656a9b419..4678c03d7 100644 --- a/libfreerdp-utils/args.c +++ b/libfreerdp-utils/args.c @@ -57,7 +57,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing color depth\n"); - return 0; + return -1; } settings->color_depth = atoi(argv[index]); } @@ -67,7 +67,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing username\n"); - return 0; + return -1; } settings->username = xstrdup(argv[index]); } @@ -77,7 +77,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing password\n"); - return 0; + return -1; } settings->password = xstrdup(argv[index]); settings->autologon = 1; @@ -95,7 +95,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing domain\n"); - return 0; + return -1; } settings->domain = xstrdup(argv[index]); } @@ -105,7 +105,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing shell\n"); - return 0; + return -1; } settings->shell = xstrdup(argv[index]); } @@ -115,7 +115,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing directory\n"); - return 0; + return -1; } settings->directory = xstrdup(argv[index]); } @@ -125,7 +125,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing dimensions\n"); - return 0; + return -1; } if (strncmp("workarea", argv[index], 1) == 0) @@ -161,7 +161,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing port number\n"); - return 0; + return -1; } settings->port = atoi(argv[index]); } @@ -171,7 +171,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing client hostname\n"); - return 0; + return -1; } strncpy(settings->client_hostname, argv[index], sizeof(settings->client_hostname) - 1); settings->client_hostname[sizeof(settings->client_hostname) - 1] = 0; @@ -227,7 +227,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing performance flag\n"); - return 0; + return -1; } if (argv[index][0] == 'm') /* modem */ { @@ -266,7 +266,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing protocol security\n"); - return 0; + return -1; } if (strncmp("rdp", argv[index], 1) == 0) /* Standard RDP */ { @@ -289,7 +289,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, else { printf("unknown protocol security\n"); - return 0; + return -1; } } else if (strcmp("--plugin", argv[index]) == 0) @@ -299,7 +299,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing plugin name\n"); - return 0; + return -1; } plugin_data = NULL; if (index < argc - 1 && strcmp("--data", argv[index + 1]) == 0) @@ -339,7 +339,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (plugin_callback != NULL) { if (!plugin_callback(settings, argv[t], plugin_data, plugin_user_data)) - return 0; + return -1; } } else if (strcmp("--ext", argv[index]) == 0) @@ -348,12 +348,12 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (index == argc) { printf("missing extension name\n"); - return 0; + return -1; } if (num_extensions >= sizeof(settings->extensions) / sizeof(struct rdp_ext_set)) { printf("maximum extensions reached\n"); - return 0; + return -1; } snprintf(settings->extensions[num_extensions].name, sizeof(settings->extensions[num_extensions].name), @@ -375,7 +375,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, else if (strcmp("--version", argv[index]) == 0) { printf("This is FreeRDP version %s\n", FREERDP_VERSION_FULL); - return 0; + return -1; } else if (argv[index][0] != '-') { @@ -416,7 +416,7 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, if (t == 0) { printf("invalid option: %s\n", argv[index]); - return 0; + return -1; } index += t - 1; } @@ -424,5 +424,5 @@ int freerdp_parse_args(rdpSettings* settings, int argc, char** argv, index++; } printf("missing server name\n"); - return 0; + return -1; } From 36e929f69c6c0ecc4f1dc5734eb36b63ec125dfc Mon Sep 17 00:00:00 2001 From: Martin Fleisz Date: Wed, 24 Aug 2011 08:07:55 -0700 Subject: [PATCH 3/3] Fixed possible socket leak in tcp_connect, added disconnect function to properly clean-up socket/tls resources on disconnect) --- client/DirectFB/dfreerdp.c | 1 + client/X11/xfreerdp.c | 1 + include/freerdp/freerdp.h | 2 ++ libfreerdp-core/freerdp.c | 9 +++++++++ libfreerdp-core/tcp.c | 14 +++++++------- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/client/DirectFB/dfreerdp.c b/client/DirectFB/dfreerdp.c index 2175a6473..e875f2b88 100644 --- a/client/DirectFB/dfreerdp.c +++ b/client/DirectFB/dfreerdp.c @@ -344,6 +344,7 @@ int dfreerdp_run(freerdp* instance) freerdp_chanman_free(chanman); df_free(dfi); gdi_free(instance); + instance->Disconnect(instance); freerdp_free(instance); return 0; diff --git a/client/X11/xfreerdp.c b/client/X11/xfreerdp.c index 2ca8aacd4..8e98d97d9 100644 --- a/client/X11/xfreerdp.c +++ b/client/X11/xfreerdp.c @@ -597,6 +597,7 @@ int xfreerdp_run(freerdp* instance) freerdp_chanman_close(chanman, instance); freerdp_chanman_free(chanman); + instance->Disconnect(instance); freerdp_free(instance); xf_free(xfi); diff --git a/include/freerdp/freerdp.h b/include/freerdp/freerdp.h index d14c6cac1..cdb758c7c 100644 --- a/include/freerdp/freerdp.h +++ b/include/freerdp/freerdp.h @@ -44,6 +44,7 @@ typedef boolean (*pcGetFileDescriptor)(freerdp* instance, void** rfds, int* rcou typedef boolean (*pcCheckFileDescriptor)(freerdp* instance); typedef int (*pcSendChannelData)(freerdp* instance, int channelId, uint8* data, int size); typedef int (*pcReceiveChannelData)(freerdp* instance, int channelId, uint8* data, int size, int flags, int total_size); +typedef void (*pcDisconnect)(freerdp* instance); struct rdp_freerdp { @@ -64,6 +65,7 @@ struct rdp_freerdp pcCheckFileDescriptor CheckFileDescriptor; pcSendChannelData SendChannelData; pcReceiveChannelData ReceiveChannelData; + pcDisconnect Disconnect; }; FREERDP_API freerdp* freerdp_new(); diff --git a/libfreerdp-core/freerdp.c b/libfreerdp-core/freerdp.c index 90969a78a..504079fc9 100644 --- a/libfreerdp-core/freerdp.c +++ b/libfreerdp-core/freerdp.c @@ -70,6 +70,14 @@ static int freerdp_send_channel_data(freerdp* instance, int channel_id, uint8* d return rdp_send_channel_data(instance->rdp, channel_id, data, size); } +void freerdp_disconnect(freerdp* instance) +{ + rdpRdp* rdp; + + rdp = (rdpRdp*) instance->rdp; + transport_disconnect(rdp->transport); +} + freerdp* freerdp_new() { freerdp* instance; @@ -88,6 +96,7 @@ freerdp* freerdp_new() instance->GetFileDescriptor = freerdp_get_fds; instance->CheckFileDescriptor = freerdp_check_fds; instance->SendChannelData = freerdp_send_channel_data; + instance->Disconnect = freerdp_disconnect; input_register_client_callbacks(rdp->input); } diff --git a/libfreerdp-core/tcp.c b/libfreerdp-core/tcp.c index 3ac693538..88c849358 100644 --- a/libfreerdp-core/tcp.c +++ b/libfreerdp-core/tcp.c @@ -102,7 +102,6 @@ void tcp_get_mac_address(rdpTcp * tcp) boolean tcp_connect(rdpTcp* tcp, const char* hostname, uint16 port) { int status; - int sockfd = -1; char servname[10]; struct addrinfo hints = { 0 }; struct addrinfo * res, * ai; @@ -119,30 +118,31 @@ boolean tcp_connect(rdpTcp* tcp, const char* hostname, uint16 port) return False; } + tcp->sockfd = -1; for (ai = res; ai; ai = ai->ai_next) { - sockfd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); + tcp->sockfd = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol); - if (sockfd < 0) + if (tcp->sockfd < 0) continue; - if (connect(sockfd, ai->ai_addr, ai->ai_addrlen) == 0) + if (connect(tcp->sockfd, ai->ai_addr, ai->ai_addrlen) == 0) { printf("connected to %s:%s\n", hostname, servname); break; } - sockfd = -1; + close(tcp->sockfd); + tcp->sockfd = -1; } freeaddrinfo(res); - if (sockfd == -1) + if (tcp->sockfd == -1) { printf("unable to connect to %s:%s\n", hostname, servname); return False; } - tcp->sockfd = sockfd; tcp_get_ip_address(tcp); tcp_get_mac_address(tcp);