maria-developers team mailing list archive

[Nirbhay Choubey <nirbhay@xxxxxxxxxxx>]

Hi Kristian,

On Fri, Oct 17, 2014 at 5:53 AM, Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>
wrote:

> Nirbhay Choubey <nirbhay@xxxxxxxxxxx> 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