Re: bk commit into 5.1 tree (hezx:1.2534) BUG#33029

[Andrei Elkin]

Zhen Xing, hello.

The patches are okay to me.

Thanks for answering my question on #rep today.

I see my remark previous remark about the default copy constructor was
not entirely correct.
I am glad the idea was accepted and the new methods plus the copy
constuctor should be a good contribution.

Great that you found yourself a slipped from my attention
rpl_master_has_bug()!

Thanks for doing it!

cheers,

Andrei

<...  removed  ...>

>> 
>> Why not to be content with the existing default copy constructor
>> instead of your new methods?
>> 
>> E.g
>> 
>>   auto_inc_intervals_forced.save(&backup->auto_inc_intervals_forced);
>> 
>> turns to be 2 instuctions
>> 
>>   backup->auto_inc_intervals_forced= auto_inc_intervals_forced;
>>   auto_inc_intervals_forced.empty_no_free();
>> 
>> but i'd rather to go with them as empty_no_free() is a public.
>> 
>
> There is no copy constructor, only a defalut constructor that takes no
> argument, correct me if I am wrong.
>
> I had thought of use the code you mentioned, the problem is I think a
> simple copy construct that just copy all the members is not safe, I mean
> after:
>
>    backup->auto_inc_intervals_forced= auto_inc_intervals_forced;
>
> We will have two instance with shared list of elements, if we use empty
> on any of the instance, the other will be in a dangling state, all the
> elements are actuall freed. In the current case, this is not a problem,
> because we can call empty_no_free to empty the original instance without
> free. But when we add a new interface, it can be use later by other
> code. So this would be an unsafe interface.
>
> If we do need the copy construct, we have to add a reference count to
> record how many copies we have, and only free the list when the count is
> zero. Maybe we should try this.
>
> BTW: I think empty_no_free should be private instead of public.
>