Re: [Modcplusplus-devel] Fix unportable and unsafe code.

[John S. <jo...@st...>]

Hi Jon -

Just getting back from vacation, sorry for the delay.  I have a queue 
of patches (some from before I left), I'll be reviewing them in order.

This patch, at a glance, looks very good.  I'll see if I can commit 
fixes tonight.  We are due for a release (other fixes have been 
committed, but not released) - so hopefully I can get everything 
applied and released early this week.

Thanks for all the great input, it really helps out.  Definitely let me 
know of any other suggestions you have, even if they may imply API 
changes.  The user base is pretty flexible (or at least HAS been in the 
past), so API changes are not out of the question.

John

On Jul 31, 2005, at 7:41 AM, Jonathan Wakely wrote:

> Hi,
>
> This patch against the latest CVS version corrects a few things I found
> that look wrong or dangerous.
>
> Firstly, in src/apache_handler.cpp if any handler thows an exception 
> then
> the object pointed to be pRequest will not be destroyed, resulting in a
> memory (and possibly other resource) leak.  This can be avoided by 
> using
> std::auto_ptr to ensure the object is destroyed.  Alternatively,
> couldn't the object be created on the stack instead of on the heap?
>
> Secondly, in src/env_value.cpp env_value::operator[] is defined to take
> a (signed) int, but vector::operator[] takes unsigned.  If a negative
> value is passed to the function then it results in undefined behaviour.
> The parameter should be unsigned, or to avoid an interface change it
> should be converted to unsigned before use.
>
> Thirdly, example/input_filter/test_input.cpp uses a printf format
> specifier of %d to print a variable of type apr_size_t, which on my
> system is a size_t, which is an unsigned 64-bit type, whereas int is a
> signed 32-bit type.  This will truncate the value and only print part 
> of
> it.  To be portable the length should be cast to unsigned long and the
> format specifier changed accordingly.
>
> Finally, example/handler/test_auth.cpp does not initialise the sent_pw
> variable, but then uses it without checking whether it was set or not.
> This can result in ap_log_error reading uninitialised memory.
>
> jon
>
> --
> "Most people would sooner die than think; in fact, they do so."
> 	- Bertrand Russell
> <mod_cplusplus-fixes.patch>