Nagios Support Forum

This is a multi-part message in MIME format.
--------------090904060002070706050503
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

Sean Millichamp wrote the following on 04.11.2009 16:07:
>
> Actually, api_query hasn't been initialized up until that point. The
> api_query pointer in your first asprintf is dependent on the results of
> the asprintf in your else clause. If you wanted to rewrite it then it
> would need to be more like:
>
> if(bare_update_check==FALSE) {
> asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1&version=%s%s",PROGRAM_VERSION,(api_query_opts==NULL)?"":api_query_opts);
> } else {
> asprintf(&api_query,"v=1&product=icinga&tinycheck=1&stableonly=1");
> }
>
> This does cause some duplication in the strings, but does otherwise look
> a bit cleaner.
Ah, yeah, I knew I missed sth on that. Thanks for clearifying! Regarding
the duplicated string, maybe an extra buf is needed.
>
> I think that is a fine solution and also much cleaner looking. I was
> trying to keep changes to the style to a minimum, but it is often hard
> to best know how to do that for a given project.
To be honest, I don't really like the style of code in there. And for
what it's worth some solutions are much more comfortable to implement
not regarding the easier reading. That's a thing coming from rewriting
IDOUtils for more RDBMs, I hesitated several times with good looking
lines but not that easy code.
>
> Which would hopefully allow the asprintf() style calls in the same
> fashion as they had been used, except substituting my_asprintf() for
> them.
asprintf is heavily used within the code so it would be a good option.
Compared to how much work it would be rewriting adn aptching that, well.
> And, yes, the current code does cause a small memory leak. As far as I
> can see, asprintf always malloc()s new memory and places the pointer in
> the first argument without using the existing value (no realloc, free,
> etc.) I found it while running valgrind on Nagios while pursuing a
> different problem and figured it was worthy of a janitorial patch.
Jep, asprintf allocated buffers need to be free'd manually. If you take
a deeper look into e.g. NDOUtils dbhandler.c you will find lots of those
statements. It's pure C but the bigger the code the more you have to
correct afterwards

Kind regards,
Michael

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

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


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







Hi,

Sean Millichamp wrote the following on 04.11.2009 16:07:


Actually, api_query hasn't been initialized up until that point. The
api_query pointer in your first asprintf is dependent on the results of
the asprintf in your else clause. If you wanted to rewrite it then it
would need to be more like:

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

This does cause some duplication in the strings, but does otherwise look
a bit cleaner.


Ah, yeah, I knew I missed sth on that. Thanks for clearifying!
Regarding the duplicated string, maybe an extra buf is needed.


I think that is a fine solution and also much cleaner looking. I was
trying to keep changes to the style to a minimum, but it is often hard
to best know how to do that for a given pro

...[email truncated]...


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