Hi Ingo,
Would this solution still demand a recompilation of any plugin that was
compiled before this change? (I.e. change of Storage Engine API, even if
not explicitly a part of the API, since those where linked against the
'hash_search' and after this change there is only 'my_hash_search',
since the define only remaps the name at compile time?).
If it changes the API, please check with Sanjay at the Storage Engine
API team.
/Mattias
Ingo Struewing wrote:
> #At file:///home2/mydev/bzrroot/mysql-5.1-bug37958/
>
> 2736 Ingo Struewing 2008-09-10
> Bug#37958 - test main.plugin crash on Mac OS X when selecting from EXAMPLE
> engine.
>
> The server crashed on Mac OS X when trying to access an EXAMPLE table
> after the EXAMPLE plugin was installed.
>
> It turned out that the dynamically loaded EXAMPLE plugin called the
> function hash_earch() from a Mac OS X system library, instead of
> hash_earch() from MySQL's mysys library. Makefile.am in storage/example
> does not include libmysys. So the Mac OS X linker arranged the hash_search()
> function to be linked to the system library when the shared object is
> loaded.
>
> One possible solution would be to include libmysys into the linkage of
> dynamic plugins. But then we must have a libmysys.so, which must be
> used by the server too. This could have a minimal performance impact,
> but foremost the change seems to bee too risky at the current state of
> MySQL 5.1.
>
> The selected solution is to rename MySQL's hash_search() to my_hash_search()
> like it has been done before with hash_insert() and hash_reset().
>
> Since this is the third time, we need to rename a hash_*() function,
> I did renamed all hash_*() functions to my_hash_*().
>
> To avoid changing a zillion calls to these functions, and announcing
> this to hundreds of developers, I added defines that map the old names
> to the new names.
> modified:
> include/hash.h
> mysys/hash.c
>
> per-file messages:
> include/hash.h
> Bug#37958 - test main.plugin crash on Mac OS X when selecting from EXAMPLE
> engine.
> Renamed hash_*() functions to my_hash_*().
> Added defines that map old names to new names.
> mysys/hash.c
> Bug#37958 - test main.plugin crash on Mac OS X when selecting from EXAMPLE
> engine.
> Renamed hash_*() functions to my_hash_*().
> === modified file 'include/hash.h'
> --- a/include/hash.h 2008-03-18 09:45:36 +0000
> +++ b/include/hash.h 2008-09-10 17:38:39 +0000
> @@ -22,6 +22,40 @@ extern "C" {
> #endif
>
> /*
> + There was a problem on MacOSX with a shared object ha_example.so.
> + It used hash_search(). During build of ha_example.so no libmysys
> + was specified. Since MacOSX had a hash_search() in the system
> + library, it built the shared object so that the dynamic linker
> + linked hash_search() to the system library, which caused a crash
> + when called. To come around this, we renamed hash_search() to
> + my_hash_search(), as we did long ago with hash_insert() and
> + hash_reset(). However, this time we made the move complete with
> + all names. To keep compatibility, we redefine the old names.
> + Since every C and C++ file, that uses HASH, needs to include
> + this file, the change is complete. Both names could be used
> + in the code, but the my_* versions are recommended now.
> +*/
> +#define hash_get_key my_hash_get_key
> +#define hash_free_key my_hash_free_key
> +#define hash_init my_hash_init
> +#define hash_init2 my_hash_init2
> +#define _hash_init _my_hash_init
> +#define hash_free my_hash_free
> +#define hash_reset my_hash_reset
> +#define hash_element my_hash_element
> +#define hash_search my_hash_search
> +#define hash_first my_hash_first
> +#define hash_next my_hash_next
> +#define hash_insert my_hash_insert
> +#define hash_delete my_hash_delete
> +#define hash_update my_hash_update
> +#define hash_replace my_hash_replace
> +#define hash_check my_hash_check
> +#define hash_clear my_hash_clear
> +#define hash_inited my_hash_inited
> +#define hash_init_opt my_hash_init_opt
> +
> +/*
> Overhead to store an element in hash
> Can be used to approximate memory consumption for a hash
> */
> @@ -30,8 +64,8 @@ extern "C" {
> /* flags for hash_init */
> #define HASH_UNIQUE 1 /* hash_insert fails on duplicate key */
>
> -typedef uchar *(*hash_get_key)(const uchar *,size_t*,my_bool);
> -typedef void (*hash_free_key)(void *);
> +typedef uchar *(*my_hash_get_key)(const uchar *,size_t*,my_bool);
> +typedef void (*my_hash_free_key)(void *);
>
> typedef struct st_hash {
> size_t key_offset,key_length; /* Length of key if const length */
> @@ -39,7 +73,7 @@ typedef struct st_hash {
> ulong records;
> uint flags;
> DYNAMIC_ARRAY array; /* Place for hash_keys */
> - hash_get_key get_key;
> + my_hash_get_key get_key;
> void (*free)(void *);
> CHARSET_INFO *charset;
> } HASH;
> @@ -47,30 +81,34 @@ typedef struct st_hash {
> /* A search iterator state */
> typedef uint HASH_SEARCH_STATE;
>
> -#define hash_init(A,B,C,D,E,F,G,H) _hash_init(A,0,B,C,D,E,F,G,H CALLER_INFO)
> -#define hash_init2(A,B,C,D,E,F,G,H,I) _hash_init(A,B,C,D,E,F,G,H,I CALLER_INFO)
> -my_bool _hash_init(HASH *hash, uint growth_size,CHARSET_INFO *charset,
> - ulong default_array_elements, size_t key_offset,
> - size_t key_length, hash_get_key get_key,
> - void (*free_element)(void*), uint flags CALLER_INFO_PROTO);
> -void hash_free(HASH *tree);
> +#define my_hash_init(A,B,C,D,E,F,G,H) \
> + _my_hash_init(A,0,B,C,D,E,F,G,H CALLER_INFO)
> +#define my_hash_init2(A,B,C,D,E,F,G,H,I) \
> + _my_hash_init(A,B,C,D,E,F,G,H,I CALLER_INFO)
> +my_bool _my_hash_init(HASH *hash, uint growth_size, CHARSET_INFO *charset,
> + ulong default_array_elements, size_t key_offset,
> + size_t key_length, my_hash_get_key get_key,
> + void (*free_element)(void*),
> + uint flags CALLER_INFO_PROTO);
> +void my_hash_free(HASH *tree);
> void my_hash_reset(HASH *hash);
> -uchar *hash_element(HASH *hash,ulong idx);
> -uchar *hash_search(const HASH *info, const uchar *key, size_t length);
> -uchar *hash_first(const HASH *info, const uchar *key, size_t length,
> - HASH_SEARCH_STATE *state);
> -uchar *hash_next(const HASH *info, const uchar *key, size_t length,
> - HASH_SEARCH_STATE *state);
> -my_bool my_hash_insert(HASH *info,const uchar *data);
> -my_bool hash_delete(HASH *hash,uchar *record);
> -my_bool hash_update(HASH *hash,uchar *record,uchar *old_key,size_t old_key_length);
> -void hash_replace(HASH *hash, HASH_SEARCH_STATE *state, uchar *new_row);
> -my_bool hash_check(HASH *hash); /* Only in debug library */
> -
> -#define hash_clear(H) bzero((char*) (H),sizeof(*(H)))
> -#define hash_inited(H) ((H)->array.buffer != 0)
> -#define hash_init_opt(A,B,C,D,E,F,G,H) \
> - (!hash_inited(A) && _hash_init(A,0,B,C,D,E,F,G, H CALLER_INFO))
> +uchar *my_hash_element(HASH *hash, ulong idx);
> +uchar *my_hash_search(const HASH *info, const uchar *key, size_t length);
> +uchar *my_hash_first(const HASH *info, const uchar *key, size_t length,
> + HASH_SEARCH_STATE *state);
> +uchar *my_hash_next(const HASH *info, const uchar *key, size_t length,
> + HASH_SEARCH_STATE *state);
> +my_bool my_hash_insert(HASH *info, const uchar *data);
> +my_bool my_hash_delete(HASH *hash, uchar *record);
> +my_bool my_hash_update(HASH *hash, uchar *record, uchar *old_key,
> + size_t old_key_length);
> +void my_hash_replace(HASH *hash, HASH_SEARCH_STATE *state, uchar *new_row);
> +my_bool my_hash_check(HASH *hash); /* Only in debug library */
> +
> +#define my_hash_clear(H) bzero((char*) (H), sizeof(*(H)))
> +#define my_hash_inited(H) ((H)->array.buffer != 0)
> +#define my_hash_init_opt(A,B,C,D,E,F,G,H) \
> + (!my_hash_inited(A) && _my_hash_init(A,0,B,C,D,E,F,G, H
> CALLER_INFO))
>
> #ifdef __cplusplus
> }
>
> === modified file 'mysys/hash.c'
> --- a/mysys/hash.c 2008-03-18 09:45:36 +0000
> +++ b/mysys/hash.c 2008-09-10 17:38:39 +0000
> @@ -33,7 +33,7 @@ typedef struct st_hash_info {
> uchar *data; /* data for current entry */
> } HASH_LINK;
>
> -static uint hash_mask(uint hashnr,uint buffmax,uint maxlength);
> +static uint my_hash_mask(uint hashnr,uint buffmax,uint maxlength);
> static void movelink(HASH_LINK *array,uint pos,uint next_link,uint newlink);
> static int hashcmp(const HASH *hash, HASH_LINK *pos, const uchar *key,
> size_t length);
> @@ -46,19 +46,19 @@ static uint calc_hash(const HASH *hash,
> }
>
> my_bool
> -_hash_init(HASH *hash,uint growth_size, CHARSET_INFO *charset,
> +_my_hash_init(HASH *hash,uint growth_size, CHARSET_INFO *charset,
> ulong size, size_t key_offset, size_t key_length,
> - hash_get_key get_key,
> + my_hash_get_key get_key,
> void (*free_element)(void*),uint flags CALLER_INFO_PROTO)
> {
> - DBUG_ENTER("hash_init");
> + DBUG_ENTER("my_hash_init");
> DBUG_PRINT("enter",("hash: 0x%lx size: %u", (long) hash, (uint) size));
>
> hash->records=0;
> if (my_init_dynamic_array_ci(&hash->array, sizeof(HASH_LINK), size,
> growth_size))
> {
> - hash->free=0; /* Allow call to hash_free */
> + hash->free=0; /* Allow call to my_hash_free */
> DBUG_RETURN(1);
> }
> hash->key_offset=key_offset;
> @@ -76,14 +76,14 @@ _hash_init(HASH *hash,uint growth_size,
> Call hash->free on all elements in hash.
>
> SYNOPSIS
> - hash_free_elements()
> + my_hash_free_elements()
> hash hash table
>
> NOTES:
> Sets records to 0
> */
>
> -static inline void hash_free_elements(HASH *hash)
> +static inline void my_hash_free_elements(HASH *hash)
> {
> if (hash->free)
> {
> @@ -100,18 +100,18 @@ static inline void hash_free_elements(HA
> Free memory used by hash.
>
> SYNOPSIS
> - hash_free()
> + my_hash_free()
> hash the hash to delete elements of
>
> - NOTES: Hash can't be reused without calling hash_init again.
> + NOTES: Hash can't be reused without calling my_hash_init again.
> */
>
> -void hash_free(HASH *hash)
> +void my_hash_free(HASH *hash)
> {
> - DBUG_ENTER("hash_free");
> + DBUG_ENTER("my_hash_free");
> DBUG_PRINT("enter",("hash: 0x%lx", (long) hash));
>
> - hash_free_elements(hash);
> + my_hash_free_elements(hash);
> hash->free= 0;
> delete_dynamic(&hash->array);
> DBUG_VOID_RETURN;
> @@ -131,7 +131,7 @@ void my_hash_reset(HASH *hash)
> DBUG_ENTER("my_hash_reset");
> DBUG_PRINT("enter",("hash: 0x%lxd", (long) hash));
>
> - hash_free_elements(hash);
> + my_hash_free_elements(hash);
> reset_dynamic(&hash->array);
> /* Set row pointers so that the hash can be reused at once */
> hash->blength= 1;
> @@ -146,7 +146,7 @@ void my_hash_reset(HASH *hash)
> */
>
> static inline char*
> -hash_key(const HASH *hash, const uchar *record, size_t *length,
> +my_hash_key(const HASH *hash, const uchar *record, size_t *length,
> my_bool first)
> {
> if (hash->get_key)
> @@ -157,18 +157,18 @@ hash_key(const HASH *hash, const uchar *
>
> /* Calculate pos according to keys */
>
> -static uint hash_mask(uint hashnr,uint buffmax,uint maxlength)
> +static uint my_hash_mask(uint hashnr,uint buffmax,uint maxlength)
> {
> if ((hashnr & (buffmax-1)) < maxlength) return (hashnr & (buffmax-1));
> return (hashnr & ((buffmax >> 1) -1));
> }
>
> -static uint hash_rec_mask(const HASH *hash, HASH_LINK *pos,
> +static uint my_hash_rec_mask(const HASH *hash, HASH_LINK *pos,
> uint buffmax, uint maxlength)
> {
> size_t length;
> - uchar *key= (uchar*) hash_key(hash,pos->data,&length,0);
> - return hash_mask(calc_hash(hash,key,length),buffmax,maxlength);
> + uchar *key= (uchar*) my_hash_key(hash,pos->data,&length,0);
> + return my_hash_mask(calc_hash(hash,key,length),buffmax,maxlength);
> }
>
>
> @@ -181,15 +181,15 @@ inline
> unsigned int rec_hashnr(HASH *hash,const uchar *record)
> {
> size_t length;
> - uchar *key= (uchar*) hash_key(hash,record,&length,0);
> + uchar *key= (uchar*) my_hash_key(hash,record,&length,0);
> return calc_hash(hash,key,length);
> }
>
>
> -uchar* hash_search(const HASH *hash, const uchar *key, size_t length)
> +uchar* my_hash_search(const HASH *hash, const uchar *key, size_t length)
> {
> HASH_SEARCH_STATE state;
> - return hash_first(hash, key, length, &state);
> + return my_hash_first(hash, key, length, &state);
> }
>
> /*
> @@ -199,17 +199,17 @@ uchar* hash_search(const HASH *hash, con
> Assigns the number of the found record to HASH_SEARCH_STATE state
> */
>
> -uchar* hash_first(const HASH *hash, const uchar *key, size_t length,
> +uchar* my_hash_first(const HASH *hash, const uchar *key, size_t length,
> HASH_SEARCH_STATE *current_record)
> {
> HASH_LINK *pos;
> uint flag,idx;
> - DBUG_ENTER("hash_first");
> + DBUG_ENTER("my_hash_first");
>
> flag=1;
> if (hash->records)
> {
> - idx=hash_mask(calc_hash(hash,key,length ? length : hash->key_length),
> + idx=my_hash_mask(calc_hash(hash,key,length ? length : hash->key_length),
> hash->blength,hash->records);
> do
> {
> @@ -223,7 +223,7 @@ uchar* hash_first(const HASH *hash, cons
> if (flag)
> {
> flag=0; /* Reset flag */
> - if (hash_rec_mask(hash,pos,hash->blength,hash->records) != idx)
> + if (my_hash_rec_mask(hash,pos,hash->blength,hash->records) != idx)
> break; /* Wrong link */
> }
> }
> @@ -234,9 +234,9 @@ uchar* hash_first(const HASH *hash, cons
> }
>
> /* Get next record with identical key */
> - /* Can only be called if previous calls was hash_search */
> + /* Can only be called if previous calls was my_hash_search */
>
> -uchar* hash_next(const HASH *hash, const uchar *key, size_t length,
> +uchar* my_hash_next(const HASH *hash, const uchar *key, size_t length,
> HASH_SEARCH_STATE *current_record)
> {
> HASH_LINK *pos;
> @@ -297,7 +297,7 @@ static int hashcmp(const HASH *hash, HAS
> size_t length)
> {
> size_t rec_keylength;
> - uchar *rec_key= (uchar*) hash_key(hash,pos->data,&rec_keylength,1);
> + uchar *rec_key= (uchar*) my_hash_key(hash,pos->data,&rec_keylength,1);
> return ((length && length != rec_keylength) ||
> my_strnncoll(hash->charset, (uchar*) rec_key, rec_keylength,
> (uchar*) key, rec_keylength));
> @@ -321,8 +321,8 @@ my_bool my_hash_insert(HASH *info,const
>
> if (HASH_UNIQUE & info->flags)
> {
> - uchar *key= (uchar*) hash_key(info, record, &idx, 1);
> - if (hash_search(info, key, idx))
> + uchar *key= (uchar*) my_hash_key(info, record, &idx, 1);
> + if (my_hash_search(info, key, idx))
> return(TRUE); /* Duplicate entry */
> }
>
> @@ -341,7 +341,7 @@ my_bool my_hash_insert(HASH *info,const
> pos=data+idx;
> hash_nr=rec_hashnr(info,pos->data);
> if (flag == 0) /* First loop; Check if ok */
> - if (hash_mask(hash_nr,info->blength,info->records) != first_index)
> + if (my_hash_mask(hash_nr,info->blength,info->records) != first_index)
> break;
> if (!(hash_nr & halfbuff))
> { /* Key will not move */
> @@ -413,7 +413,7 @@ my_bool my_hash_insert(HASH *info,const
> }
> /* Check if we are at the empty position */
>
> - idx=hash_mask(rec_hashnr(info,record),info->blength,info->records+1);
> + idx=my_hash_mask(rec_hashnr(info,record),info->blength,info->records+1);
> pos=data+idx;
> if (pos == empty)
> {
> @@ -424,7 +424,7 @@ my_bool my_hash_insert(HASH *info,const
> {
> /* Check if more records in same hash-nr family */
> empty[0]=pos[0];
> - gpos=data+hash_rec_mask(info,pos,info->blength,info->records+1);
> + gpos=data+my_hash_rec_mask(info,pos,info->blength,info->records+1);
> if (pos == gpos)
> {
> pos->data=(uchar*) record;
> @@ -449,18 +449,18 @@ my_bool my_hash_insert(HASH *info,const
> ** if there is a free-function it's called for record if found
> ******************************************************************************/
>
> -my_bool hash_delete(HASH *hash,uchar *record)
> +my_bool my_hash_delete(HASH *hash,uchar *record)
> {
> uint blength,pos2,pos_hashnr,lastpos_hashnr,idx,empty_index;
> HASH_LINK *data,*lastpos,*gpos,*pos,*pos3,*empty;
> - DBUG_ENTER("hash_delete");
> + DBUG_ENTER("my_hash_delete");
> if (!hash->records)
> DBUG_RETURN(1);
>
> blength=hash->blength;
> data=dynamic_element(&hash->array,0,HASH_LINK*);
> /* Search after record with key */
> - pos=data+ hash_mask(rec_hashnr(hash,record),blength,hash->records);
> + pos=data+ my_hash_mask(rec_hashnr(hash,record),blength,hash->records);
> gpos = 0;
>
> while (pos->data != record)
> @@ -491,7 +491,7 @@ my_bool hash_delete(HASH *hash,uchar *re
> /* Move the last key (lastpos) */
> lastpos_hashnr=rec_hashnr(hash,lastpos->data);
> /* pos is where lastpos should be */
> - pos=data+hash_mask(lastpos_hashnr,hash->blength,hash->records);
> + pos=data+my_hash_mask(lastpos_hashnr,hash->blength,hash->records);
> if (pos == empty) /* Move to empty position. */
> {
> empty[0]=lastpos[0];
> @@ -499,7 +499,7 @@ my_bool hash_delete(HASH *hash,uchar *re
> }
> pos_hashnr=rec_hashnr(hash,pos->data);
> /* pos3 is where the pos should be */
> - pos3= data+hash_mask(pos_hashnr,hash->blength,hash->records);
> + pos3= data+my_hash_mask(pos_hashnr,hash->blength,hash->records);
> if (pos != pos3)
> { /* pos is on wrong posit */
> empty[0]=pos[0]; /* Save it here */
> @@ -507,8 +507,8 @@ my_bool hash_delete(HASH *hash,uchar *re
> movelink(data,(uint) (pos-data),(uint) (pos3-data),empty_index);
> goto exit;
> }
> - pos2= hash_mask(lastpos_hashnr,blength,hash->records+1);
> - if (pos2 == hash_mask(pos_hashnr,blength,hash->records+1))
> + pos2= my_hash_mask(lastpos_hashnr,blength,hash->records+1);
> + if (pos2 == my_hash_mask(pos_hashnr,blength,hash->records+1))
> { /* Identical key-positions */
> if (pos2 != hash->records)
> {
> @@ -536,26 +536,26 @@ exit:
> This is much more efficent than using a delete & insert.
> */
>
> -my_bool hash_update(HASH *hash, uchar *record, uchar *old_key,
> +my_bool my_hash_update(HASH *hash, uchar *record, uchar *old_key,
> size_t old_key_length)
> {
> uint new_index,new_pos_index,blength,records,empty;
> size_t idx;
> HASH_LINK org_link,*data,*previous,*pos;
> - DBUG_ENTER("hash_update");
> + DBUG_ENTER("my_hash_update");
>
> if (HASH_UNIQUE & hash->flags)
> {
> HASH_SEARCH_STATE state;
> - uchar *found, *new_key= (uchar*) hash_key(hash, record, &idx, 1);
> - if ((found= hash_first(hash, new_key, idx, &state)))
> + uchar *found, *new_key= (uchar*) my_hash_key(hash, record, &idx, 1);
> + if ((found= my_hash_first(hash, new_key, idx, &state)))
> {
> do
> {
> if (found != record)
> DBUG_RETURN(1); /* Duplicate entry */
> }
> - while ((found= hash_next(hash, new_key, idx, &state)));
> + while ((found= my_hash_next(hash, new_key, idx, &state)));
> }
> }
>
> @@ -564,11 +564,11 @@ my_bool hash_update(HASH *hash, uchar *r
>
> /* Search after record with key */
>
> - idx=hash_mask(calc_hash(hash, old_key,(old_key_length ?
> + idx=my_hash_mask(calc_hash(hash, old_key,(old_key_length ?
> old_key_length :
> hash->key_length)),
> blength,records);
> - new_index=hash_mask(rec_hashnr(hash,record),blength,records);
> + new_index=my_hash_mask(rec_hashnr(hash,record),blength,records);
> if (idx == new_index)
> DBUG_RETURN(0); /* Nothing to do (No record check) */
> previous=0;
> @@ -618,7 +618,7 @@ my_bool hash_update(HASH *hash, uchar *r
> DBUG_RETURN(0);
> }
> pos=data+new_index;
> - new_pos_index=hash_rec_mask(hash,pos,blength,records);
> + new_pos_index=my_hash_rec_mask(hash,pos,blength,records);
> if (new_index != new_pos_index)
> { /* Other record in wrong position */
> data[empty] = *pos;
> @@ -636,7 +636,7 @@ my_bool hash_update(HASH *hash, uchar *r
> }
>
>
> -uchar *hash_element(HASH *hash,ulong idx)
> +uchar *my_hash_element(HASH *hash,ulong idx)
> {
> if (idx < hash->records)
> return dynamic_element(&hash->array,idx,HASH_LINK*)->data;
> @@ -649,7 +649,7 @@ uchar *hash_element(HASH *hash,ulong idx
> isn't changed
> */
>
> -void hash_replace(HASH *hash, HASH_SEARCH_STATE *current_record, uchar *new_row)
> +void my_hash_replace(HASH *hash, HASH_SEARCH_STATE *current_record, uchar *new_row)
> {
> if (*current_record != NO_RECORD) /* Safety */
> dynamic_element(&hash->array, *current_record, HASH_LINK*)->data=
> new_row;
> @@ -658,7 +658,7 @@ void hash_replace(HASH *hash, HASH_SEARC
>
> #ifndef DBUG_OFF
>
> -my_bool hash_check(HASH *hash)
> +my_bool my_hash_check(HASH *hash)
> {
> int error;
> uint i,rec_link,found,max_links,seek,links,idx;
> @@ -671,7 +671,7 @@ my_bool hash_check(HASH *hash)
>
> for (i=found=max_links=seek=0 ; i < records ; i++)
> {
> - if (hash_rec_mask(hash,data+i,blength,records) == i)
> + if (my_hash_rec_mask(hash,data+i,blength,records) == i)
> {
> found++; seek++; links=1;
> for (idx=data[i].next ;
> @@ -687,7 +687,7 @@ my_bool hash_check(HASH *hash)
> }
> hash_info=data+idx;
> seek+= ++links;
> - if ((rec_link=hash_rec_mask(hash,hash_info,blength,records)) != i)
> + if ((rec_link=my_hash_rec_mask(hash,hash_info,blength,records)) != i)
> {
> DBUG_PRINT("error",
> ("Record in wrong link at %d: Start %d Record: 0x%lx Record-link %d",
>
>
