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.
>
