Hi Marko,
Some minor requests and suggestions below. Otherwise, patch approved.
On 5/19/10 11:32 AM, marko.makela@stripped wrote:
> #At file:///home/marko/innobase/dev/mysql/5.1-innodb/ based on
> revid:marko.makela@stripped
>
> 3474 Marko Mäkelä 2010-05-19
> Implement Valgrind memcheck instrumentation in the MySQL core. (Bug #53593)
We usually use the syntax:
Bug#nnnnn: Title
in the first line. There are tools that use this information to control
the bug state -- like, move it to Documenting once it is merged to one
of the main trees.
> BUILD/*: Add valgrind_configs=--with-valgrind.
> BUILD/*: Remove -USAFEMALLOC from valgrind_flags.
>
[..]
> === modified file 'include/my_sys.h'
> --- a/include/my_sys.h revid:marko.makela@stripped
> +++ b/include/my_sys.h revid:marko.makela@stripped
> @@ -25,6 +25,27 @@ typedef struct my_aio_result {
> } my_aio_result;
> #endif
>
> +#ifdef HAVE_VALGRIND
> +# include<valgrind/memcheck.h>
> +# define MEM_UNDEFINED(a,len) VALGRIND_MAKE_MEM_UNDEFINED(a,len)
> +# define MEM_NOACCESS(a,len) VALGRIND_MAKE_MEM_NOACCESS(a,len)
> +# define MEM_DEFINED_ADDRESSABLE(a,len) VALGRIND_MAKE_MEM_DEFINED(a,len)
> +# define MEM_DEFINED(a,len) VALGRIND_MAKE_MEM_DEFINED_IF_ADDRESSABLE(a,len)
> +# define MEM_GET_VBITS(a,v,len) VALGRIND_GET_VBITS(a,v,len)
> +# define MEM_SET_VBITS(a,v,len) VALGRIND_SET_VBITS(a,v,len)
> +# define MEM_CHECK_ADDRESSABLE(a,len) VALGRIND_CHECK_MEM_IS_ADDRESSABLE(a,len)
> +# define MEM_CHECK_DEFINED(a,len) VALGRIND_CHECK_MEM_IS_DEFINED(a,len)
> +#else /* HAVE_VALGRIND */
> +# define MEM_UNDEFINED(a,len) ((void) 0)
> +# define MEM_NOACCESS(a,len) ((void) 0)
> +# define MEM_DEFINED_ADDRESSABLE(a,len) ((void) 0)
> +# define MEM_DEFINED(a,len) ((void) 0)
> +# define MEM_GET_VBITS(a,v,len) ((void) 0)
> +# define MEM_SET_VBITS(a,v,len) ((void) 0)
> +# define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0)
> +# define MEM_CHECK_DEFINED(a,len) ((void) 0)
> +#endif /* HAVE_VALGRIND */
Do you plan to use all of those soon? If no, please only keep those that
are likely to be used in the near future.
> #ifndef THREAD
> extern int NEAR my_errno; /* Last error in mysys */
> #else
> @@ -141,7 +162,7 @@ extern int NEAR my_errno; /* Last error
> #define my_memdup(A,B,C) _my_memdup((A),(B), __FILE__,__LINE__,C)
> #define my_strdup(A,C) _my_strdup((A), __FILE__,__LINE__,C)
> #define my_strndup(A,B,C) _my_strndup((A),(B),__FILE__,__LINE__,C)
> -#define TRASH(A,B) bfill(A, B, 0x8F)
> +#define TRASH(A,B) do { bfill(A, B, 0x8F); MEM_UNDEFINED(A, B); } while (0)
> #define QUICK_SAFEMALLOC sf_malloc_quick=1
> #define NORMAL_SAFEMALLOC sf_malloc_quick=0
> extern uint sf_malloc_prehunc,sf_malloc_endhunc,sf_malloc_quick;
> @@ -169,7 +190,10 @@ extern char *my_strndup(const char *from
> #define CALLER_INFO_PROTO /* nothing */
> #define CALLER_INFO /* nothing */
> #define ORIG_CALLER_INFO /* nothing */
> -#define TRASH(A,B) /* nothing */
> +#define TRASH(A,B) do { \
> + MEM_CHECK_ADDRESSABLE(A, B); \
> + MEM_UNDEFINED(A, B); \
> +} while (0)
No tabs in MySQL code.
http://forge.mysql.com/wiki/MySQL_Internals_Coding_Guidelines
Otherwise looks good. Thanks for working on this!
Regards,
--
Davi Arnaut
