Hi Andrei!
Patch approved, but see my comments below.
Just my few cents,
Mats Kindahl
Andrei Elkin wrote:
[snip]
>
> +/**
> + A comparison function to be supplied as argument to @c sort_dynamic()
> + and @c bsearch()
> +
> + @return -1 if first argument is less, 0 if it equal to, 1 if it is greater
> + than the second
> +*/
> +int change_master_server_id_cmp(ulong *id1, ulong *id2)
> +{
> + return *id1 < *id2? -1 : (*id1 > *id2? 1 : 0);
> +}
I usually prefer to put the casts inside the comparison function and not
use a cast when passing the function. However, you cannot do that since
the two comparison functions have different signatures.
> +
> +
> +/**
> + Reports if the s_id server has been configured to ignore events
> + it generates with
> +
> + CHANGE MASTER IGNORE_SERVER_IDS= ( list of server ids )
> +
> + Method is called from the io thread event receiver filtering.
> +
> + @param s_id the master server identifier
> +
> + @retval TRUE if s_id is in the list of ignored master servers,
> + @retval FALSE otherwise.
> + */
> +bool Master_info::shall_ignore_server_id(ulong s_id)
> +{
> + if (likely(ignore_server_ids.elements == 1))
> + return (* (ulong*) dynamic_array_ptr(&ignore_server_ids, 0)) == s_id;
> + else
> + return bsearch((const ulong *) &s_id,
You don't need this cast, casts to (void*) can be done implicitly. Casts
are evil: they hide errors are are almost never needed, assuming
surrounding code is written correctly.
> + ignore_server_ids.buffer,
> + ignore_server_ids.elements, sizeof(ulong),
> + (int (*) (const void*, const void*))
> change_master_server_id_cmp)
Here, I would prefer to not have an explicit cast, but that cannot be
easily accommodated since the comparison functions for DYNAMIC_ARRAY and
bsearch() have different signatures.
[snip]
> + cur_len +=my_sprintf(ignore_server_ids_buf + cur_len,
space is on wrong side of +=
[snip]
> + if (bsearch((const ulong *) &s_id,
> + mi->ignore_server_ids.buffer,
> + mi->ignore_server_ids.elements, sizeof(ulong),
> + (int (*) (const void*, const void*))
> + change_master_server_id_cmp) == NULL)
> + insert_dynamic(&mi->ignore_server_ids, (uchar*) &s_id);
> + }
> + }
> + sort_dynamic(&mi->ignore_server_ids, (qsort_cmp)
> change_master_server_id_cmp);
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
