Hi!
"knielsen" == knielsen <knielsen@knielsen-hq.org> writes:
knielsen> The sql_string.h String class has some strange code to null-terminate knielsen> the string for String::c_ptr(). knielsen> It checks if the string is already null-terminated, and only writes knielsen> the '\0' at the end if it is not already there. knielsen> The reason for this I think is to avoid the re-allocation of the buffer to fit the null termination at append time if it is not needed due to never calling c_ptr(). Yes. knielsen> However, the problem case here is a different one, where the null knielsen> termination fits in the given buffer, but the value is uninitialised. In what code did this happen ? In most of MySQL code we will put a \0 at end when we generate a string. knielsen> In this case, checking before zeroing works (if in a somewhat twisted knielsen> way). But it causes a false Valgrind alarm. knielsen> This is a minimal fix; not sure if it is the right one. Maybe it would knielsen> be better to fix the c_str() method to not use this tricky way of knielsen> working? The problem with fixing c_ptr() is that we would have to do a lot of unnecessary mallocs; This is in particular true when you use a String as a pointer to a null terminated C string. In this case the end \0 is not part of the string and a c_ptr() would force us to do a not-needed malloc. knielsen> --- knielsen> sql/sql_string.h | 8 ++++++++ knielsen> 1 file changed, 8 insertions(+) knielsen> Index: work-5.1-buildbot/sql/sql_string.h knielsen> =================================================================== knielsen> --- work-5.1-buildbot.orig/sql/sql_string.h 2009-04-08 00:34:49.000000000 +0200 knielsen> +++ work-5.1-buildbot/sql/sql_string.h 2009-04-08 00:35:38.000000000 +0200 knielsen> @@ -99,7 +99,15 @@ public: knielsen> inline const char *ptr() const { return Ptr; } knielsen> inline char *c_ptr() knielsen> { knielsen> + /* knielsen> + This code null-terminates the string if not already null-terminated. knielsen> + This works whether Ptr[str_length] is undefined or not, but if undefined knielsen> + it generates a Valgrind error. So in the Valgrind case, null-terminate knielsen> + unconditionally. knielsen> + */ knielsen> +#ifndef HAVE_purify knielsen> if (!Ptr || Ptr[str_length]) /* Should be safe */ knielsen> +#endif knielsen> (void) realloc(str_length); knielsen> return Ptr; knielsen> } In this case I would like to keep the original code to ensure we don't access unallocated or uninitialized memory. Regards, Monty