Page 1 of 1

Re: [Nagios-devel] [PATCH] Fix some minor memory leaks

Posted: Wed Nov 04, 2009 2:18 pm
by Guest
This is a multi-part message in MIME format.
--------------060305070808000907040405
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

Sean Millichamp wrote the following on 02.11.2009 18:39:
> There are some instances where asprintf() are used to construct strings
> but the resulting allocated memory isn't ever free()d. This patch
> (against Nagios 3.2.0) makes sure that the strings allocated by asprintf
> are freed when they are no longer needed.
For the single free statements I fully agree with the implementation.
But for the longer parts I would recommend another way of implementation
- I think your direction is kind of complicated, no offense.

/* generate the query */
asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1");
if(bare_update_check==FALSE)
asprintf(&api_query,"%s&version=%s%s",api_query,PROGRAM_VERSION,(api_query_opts==NULL)?"":api_query_opts);

rewritten to

|/* generate the query */

if(bare_update_check==FALSE) {
asprintf(&api_query,"%s&version=%s%s",api_query,PROGRAM_VERSION,(api_query_opts==NULL)?"":api_query_opts);
} else {
asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1");
}|

does the same, but does not need a second buffer and needs less cycles
as your proposal.

same for

asprintf(&buf,"POST %s HTTP/1.0\r\n",api_path);
asprintf(&buf,"%sUser-Agent: Nagios/%s\r\n",buf,PROGRAM_VERSION);
asprintf(&buf,"%sConnection: close\r\n",buf);
asprintf(&buf,"%sHost: %s\r\n",buf,api_server);
asprintf(&buf,"%sContent-Type: application/x-www-form-urlencoded\r\n",buf);
asprintf(&buf,"%sContent-Length: %d\r\n",buf,strlen(api_query));
asprintf(&buf,"%s\r\n",buf);
asprintf(&buf,"%s%s\r\n",buf,api_query);

if this causes a real memory leak, put it alltogether in one single line
and buffer.

asprintf(&buf,"POST %s HTTP/1.0\r\nUser-Agent: Nagios/%s\r\nConnection: close\r\nHost: %s\r\nContent-Type: application/x-www-form-urlencoded\r\nContent-Length: %d\r\n\r\n%s\r\n",api_path, PROGRAM_VERSION, api_server, strlen(api_query), api_query);

if you need better reading then just reformat the string in the code a bit.

Your opinions on that?

Kind regards,
Michael

--
DI (FH) Michael Friedrich
[email protected]
Tel: +43 1 4277 14359

Vienna University Computer Center
Universitaetsstrasse 7
A-1010 Vienna, Austria


--------------060305070808000907040405
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit







Hi,

Sean Millichamp wrote the following on 02.11.2009 18:39:

There are some instances where asprintf() are used to construct strings
but the resulting allocated memory isn't ever free()d. This patch
(against Nagios 3.2.0) makes sure that the strings allocated by asprintf
are freed when they are no longer needed.


For the single free statements I fully agree with the
implementation. But for the longer parts I would recommend another way
of implementation - I think your direction is kind of complicated, no
offense.


/* generate the query */
asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1");
if(bare_update_check==FALSE)
asprintf(&api_query,"%s&version=%s%s",api_query,PROGRAM_VERSION,(api_query_opts==NULL)?"":api_query_opts);

rewritten to
/* generate the query */

if(bare_update_check==FALSE) {
asprintf(&api_query,"%s&version=%s%s",api_query,PROGRAM_VERSION,(api_query_opts==NULL)?"":api_query_opts);
} else {
asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1");
}
does the same, but does not need a second buffer and needs less
cycles as your proposal.

same for
asprintf(&

...[email truncated]...


This post was automatically imported from historical nagios-devel mailing list archives
Original poster: [email protected]