From: Michael Widenius Date: October 14 2009 4:13pm Subject: re: String:c_ptr makes me unhappy List-Archive: http://lists.mysql.com/internals/37372 Message-Id: <19157.63657.390558.64101@narttu.askmonty.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Hi! >>>>> "MARK" == MARK CALLAGHAN writes: MARK> Have you wasted time on valgrind warnings from code that uses MARK> String:c_ptr instead of String:c_ptr_safe? MARK> Have you fixed bugs caused by the use of String:c_ptr instead of MARK> String::c_ptr_safe? MARK> I have and I am not happy about it. Over on another mailing list I MARK> read about another bug caused by using c_ptr instead of c_ptr_safe. It MARK> wasn't Drizzle, as they appear to have made c_ptr do the right thing. MARK> There are a few changes that can improve this: MARK> 1) Add comments to sql_string.h. There are none today in 5.0 and 5.1 MARK> in sql_string.h for c_ptr() and c_ptr_unsafe(). I know that using MARK> comments in header files rather than source files has been MARK> controversial in MySQL. AFAIK, that issue is limited to MySQL or ctags MARK> users within MySQL devel. I have never encountered a project where MARK> that wasn't done or at least considered the right thing to do. The main reason for me personally having comments in the code instead of the header file, is that in the .c and .cc files you can have much more description about the function and in practice I never use the .h files to look for things. However, other peoples way to work differ and I can see a good reason to provide some documentation in the .h file too. MARK> 2) Change the name of c_ptr as it doesn't return a C pointer to a MARK> string. If it did, the result would be nul terminated and there would MARK> be no need for c_ptr_unsafe. c_ptr does return a pointer to a \0 terminated string. It does however don't know if the last byte is initialized memory or not, which is a problem for valgrind but not normally for MySQL as Strings is mainly used for thread specific memory. There is no c_ptr_unsafe, only c_ptr_safe. The reason to have both version is the few cases where we know that the String is not in thread specific memory and it's worth the extra malloc + copy to access the string. In most cases the proper solution is of course to get rid of all c-terminated strings, in which case we don't have a problem anymore... Regards, Monty