* [Qemu-devel] [PATCH v2] libcacard: fix resource leak
@ 2014-11-14 2:18 zhanghailiang
2014-11-14 9:29 ` Markus Armbruster
0 siblings, 1 reply; 3+ messages in thread
From: zhanghailiang @ 2014-11-14 2:18 UTC (permalink / raw)
To: qemu-trivial; +Cc: zhanghailiang, mjt, qemu-devel, peter.huangpeng
In function connect_to_qemu(), getaddrinfo() will allocate memory
that is stored into server, it should be freed by using freeaddrinfo()
before connect_to_qemu() return.
Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
v2:
- fix typo in title ;)
---
libcacard/vscclient.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index 80111df..fa6041d 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -597,7 +597,7 @@ connect_to_qemu(
const char *port
) {
struct addrinfo hints;
- struct addrinfo *server;
+ struct addrinfo *server = NULL;
int ret, sock;
sock = socket(AF_INET, SOCK_STREAM, 0);
@@ -629,9 +629,14 @@ connect_to_qemu(
if (verbose) {
printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader));
}
+
+ freeaddrinfo(server);
return sock;
cleanup_socket:
+ if (server) {
+ freeaddrinfo(server);
+ }
closesocket(sock);
return -1;
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] libcacard: fix resource leak
2014-11-14 2:18 [Qemu-devel] [PATCH v2] libcacard: fix resource leak zhanghailiang
@ 2014-11-14 9:29 ` Markus Armbruster
2014-11-17 5:18 ` zhanghailiang
0 siblings, 1 reply; 3+ messages in thread
From: Markus Armbruster @ 2014-11-14 9:29 UTC (permalink / raw)
To: zhanghailiang; +Cc: qemu-trivial, mjt, qemu-devel, peter.huangpeng
zhanghailiang <zhang.zhanghailiang@huawei.com> writes:
> In function connect_to_qemu(), getaddrinfo() will allocate memory
> that is stored into server, it should be freed by using freeaddrinfo()
> before connect_to_qemu() return.
>
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
> v2:
> - fix typo in title ;)
> ---
> libcacard/vscclient.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
> index 80111df..fa6041d 100644
> --- a/libcacard/vscclient.c
> +++ b/libcacard/vscclient.c
> @@ -597,7 +597,7 @@ connect_to_qemu(
> const char *port
> ) {
> struct addrinfo hints;
> - struct addrinfo *server;
> + struct addrinfo *server = NULL;
> int ret, sock;
>
> sock = socket(AF_INET, SOCK_STREAM, 0);
> @@ -629,9 +629,14 @@ connect_to_qemu(
> if (verbose) {
> printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader));
> }
> +
> + freeaddrinfo(server);
> return sock;
>
> cleanup_socket:
> + if (server) {
> + freeaddrinfo(server);
> + }
> closesocket(sock);
> return -1;
> }
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Aside: this code uses the first result from getaddrinfo(), and fails if
it can't connect. This is a common misuse of getaddrinfo().
Consider a remote host with both an IPv4 and an IPv6 address, but only
one of them actually connects. If getaddrinfo() puts the connectable
one first, we succeed. Else, we fail.
We should try the addresses in order until connect() succeeds, like
qemu-sockets.c does.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH v2] libcacard: fix resource leak
2014-11-14 9:29 ` Markus Armbruster
@ 2014-11-17 5:18 ` zhanghailiang
0 siblings, 0 replies; 3+ messages in thread
From: zhanghailiang @ 2014-11-17 5:18 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-trivial, mjt, qemu-devel, peter.huangpeng
On 2014/11/14 17:29, Markus Armbruster wrote:
> zhanghailiang <zhang.zhanghailiang@huawei.com> writes:
>
>> In function connect_to_qemu(), getaddrinfo() will allocate memory
>> that is stored into server, it should be freed by using freeaddrinfo()
>> before connect_to_qemu() return.
>>
>> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
>> ---
>> v2:
>> - fix typo in title ;)
>> ---
>> libcacard/vscclient.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
>> index 80111df..fa6041d 100644
>> --- a/libcacard/vscclient.c
>> +++ b/libcacard/vscclient.c
>> @@ -597,7 +597,7 @@ connect_to_qemu(
>> const char *port
>> ) {
>> struct addrinfo hints;
>> - struct addrinfo *server;
>> + struct addrinfo *server = NULL;
>> int ret, sock;
>>
>> sock = socket(AF_INET, SOCK_STREAM, 0);
>> @@ -629,9 +629,14 @@ connect_to_qemu(
>> if (verbose) {
>> printf("Connected (sizeof Header=%zd)!\n", sizeof(VSCMsgHeader));
>> }
>> +
>> + freeaddrinfo(server);
>> return sock;
>>
>> cleanup_socket:
>> + if (server) {
>> + freeaddrinfo(server);
>> + }
>> closesocket(sock);
>> return -1;
>> }
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Aside: this code uses the first result from getaddrinfo(), and fails if
> it can't connect. This is a common misuse of getaddrinfo().
>
> Consider a remote host with both an IPv4 and an IPv6 address, but only
> one of them actually connects. If getaddrinfo() puts the connectable
> one first, we succeed. Else, we fail.
>
> We should try the addresses in order until connect() succeeds, like
> qemu-sockets.c does.
Good catch, i will fix it soon. And this patch mainly fix the coverity warning.
Maybe i should fix the problem after this patch been merged. ;) Thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-11-17 5:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-14 2:18 [Qemu-devel] [PATCH v2] libcacard: fix resource leak zhanghailiang
2014-11-14 9:29 ` Markus Armbruster
2014-11-17 5:18 ` zhanghailiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).