Re: [Maria-developers] [Commits] Rev 3900: Fix for test failures on 64-bit platform. in lp:~maria-captains/maria/10.0-galera-6593
Nirbhay Choubey <nirbhay@mariadb.com> writes:
message: Fix for test failures on 64-bit platform. === modified file 'sql/rpl_mi.cc' --- a/sql/rpl_mi.cc 2014-10-10 23:06:40 +0000 +++ b/sql/rpl_mi.cc 2014-10-17 02:08:55 +0000 @@ -1466,7 +1466,7 @@ { for (int i= DO_DOMAIN_IDS; i <= IGNORE_DOMAIN_IDS; i ++) { - my_init_dynamic_array(&m_domain_ids[i], sizeof(uint32), 16, 16, MYF(0)); + my_init_dynamic_array(&m_domain_ids[i], sizeof(ulong), 16, 16, MYF(0)); } }
Why do you need to use ulong? Domain ids should always be uint32, also on 64-bit. What is the real problem? Eg. why is there a test failure? I'd expect that the right fix was to convert to uint32 in the places where a domain_id might come into the code as ulong...
@@ -1483,7 +1483,7 @@ domain ids list. DO_DOMAIN_IDS list is only looked-up is both (do & ignore) list are non-empty. */ -void Domain_id_filter::do_filter(uint32 domain_id) +void Domain_id_filter::do_filter(ulong domain_id) { DYNAMIC_ARRAY *do_domain_ids= &m_domain_ids[DO_DOMAIN_IDS]; DYNAMIC_ARRAY *ignore_domain_ids= &m_domain_ids[IGNORE_DOMAIN_IDS]; @@ -1491,21 +1491,21 @@ if (do_domain_ids->elements > 0) { if (likely(do_domain_ids->elements == 1)) - m_filter= ((* (uint32*) dynamic_array_ptr(do_domain_ids, 0)) + m_filter= ((* (ulong *) dynamic_array_ptr(do_domain_ids, 0)) != domain_id); else - m_filter= (bsearch((const uint32 *) &domain_id, do_domain_ids->buffer, - do_domain_ids->elements, sizeof(uint32), + m_filter= (bsearch((const ulong *) &domain_id, do_domain_ids->buffer, + do_domain_ids->elements, sizeof(ulong),
Why is a cast needed here? If it's because of const, could you avoid the cast just by declaring domain_id const in the argument of the function? Thanks, - Kristian.
Hi Kristian, On Fri, Oct 17, 2014 at 5:53 AM, Kristian Nielsen <knielsen@knielsen-hq.org> wrote:
Nirbhay Choubey <nirbhay@mariadb.com> writes:
message: Fix for test failures on 64-bit platform. === modified file 'sql/rpl_mi.cc' --- a/sql/rpl_mi.cc 2014-10-10 23:06:40 +0000 +++ b/sql/rpl_mi.cc 2014-10-17 02:08:55 +0000 @@ -1466,7 +1466,7 @@ { for (int i= DO_DOMAIN_IDS; i <= IGNORE_DOMAIN_IDS; i ++) { - my_init_dynamic_array(&m_domain_ids[i], sizeof(uint32), 16, 16, MYF(0)); + my_init_dynamic_array(&m_domain_ids[i], sizeof(ulong), 16, 16, MYF(0)); } }
Why do you need to use ulong? Domain ids should always be uint32, also on 64-bit.
What is the real problem? Eg. why is there a test failure?
It was due to the different sizes for ulong on different platforms. While trying to reuse the (modified) auxiliary functions for IGNORE_SERVER_IDS where ulong is used for ids (server_id), uint32 store for domain_id did not work on 64-bit. To solve this, templates did not work well, so was left with either changing the global server_id to uint32 (which seem right to me as the valid range for @@server_id is (0, UINT_MAX32)) or using ulong in domain id filtering implementation, which would work for both 32/64 archs. I chose later to not offset the existing code. I'd expect that the
right fix was to convert to uint32 in the places where a domain_id might come into the code as ulong...
Sorry, I did not follow that.
@@ -1483,7 +1483,7 @@ domain ids list. DO_DOMAIN_IDS list is only looked-up is both (do & ignore) list are non-empty. */ -void Domain_id_filter::do_filter(uint32 domain_id) +void Domain_id_filter::do_filter(ulong domain_id) { DYNAMIC_ARRAY *do_domain_ids= &m_domain_ids[DO_DOMAIN_IDS]; DYNAMIC_ARRAY *ignore_domain_ids= &m_domain_ids[IGNORE_DOMAIN_IDS]; @@ -1491,21 +1491,21 @@ if (do_domain_ids->elements > 0) { if (likely(do_domain_ids->elements == 1)) - m_filter= ((* (uint32*) dynamic_array_ptr(do_domain_ids, 0)) + m_filter= ((* (ulong *) dynamic_array_ptr(do_domain_ids, 0)) != domain_id); else - m_filter= (bsearch((const uint32 *) &domain_id, do_domain_ids->buffer, - do_domain_ids->elements, sizeof(uint32), + m_filter= (bsearch((const ulong *) &domain_id, do_domain_ids->buffer, + do_domain_ids->elements, sizeof(ulong),
Why is a cast needed here? If it's because of const, could you avoid the cast just by declaring domain_id const in the argument of the function?
Right cast isn't really needed here, the param should be const instead. will update. Thanks, Nirbhay
participants (2)
-
Kristian Nielsen -
Nirbhay Choubey