Re: [Maria-developers] [Commits] Rev 3991: MDEV-5138 Numerous test failures in mtr --ps --embedded
Hi, Alexey! On Apr 02, Alexey Botchkov wrote: Still, another question, why did you have to rename net_store_data() to net_store_data_cs() ? That's personal. I really don't like functions with exactly same names - makes code harder to understand and debug. Do you belive it's better not to rename these here? That's up to you. Sometimes I create functions with the same name, sometimes I don't. For example, tdc_acquire_share() - there are few functions with the same name, because they do exactly the same. Actually, one of tdc_acquire_share() is a real function, others are inline convenience wrappers - they eventually call the real one. But tdc_acquire_share_shortlived() - while also only a convenience wrapper - has a different name to emphasize that it has different semantics. But anyway, that's your patch. If you'd like to rename - go ahead. Regards, Sergei ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Commits] Rev 3991: MDEV-5138 Numerous test failures in mtr --ps --embedded
Hi, Alexey!
On Mar 26, Alexey Botchkov wrote:
Hi, Sergei.
The Protocol::net_store_data is virtual in the embedded-server case
because we need it implemented differently for Protocol_binary.
Ok, got it (finally), sorry.
I'm almost always use ctags when looking for definitions, but in this
case ctags didn't handle this correctly:
#ifndef EMBEDDED_LIBRARY
bool Protocol::net_store_data(const uchar *from, size_t length)
#else
bool Protocol_binary::net_store_data(const uchar *from, size_t length)
#endif
{
...
so I never noticed that Protocol_binary actually has its own
implementation of ::net_store_data() in the embedded case.
Still, another question, why did you have to rename net_store_data() to
net_store_data_cs() ? I've simply made it virtual and everything compiled
just fine - it has a different signature so the compiler can distinguish
them all right.
Regards,
Sergei
___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Commits] Rev 3991: MDEV-5138 Numerous test failures in mtr --ps --embedded
Hi, Sergei. Why does Protocol::net_store_data() have to be virtual? Well it doen't have to be virtual, but it's the nicest solution i came up with. All the related code is in the sql/protocol.cc In fact only reason is the Protocol::store_string_aux() function that calls the ::net_store_data() internally. Both Protocol_text::store() and Protocol_binary::store() in turn call the Protocol::store_string_aux(). As the ::net_store_data() has to work differently for Protocol_binary and Protocol_text, I saw options either to create two absolutely same Protocol_text::store_string_aux() and Protocol_binary::store_string_aux() or make the net_store_data() virtual. Best regards. HF 18.03.2014 20:24, Sergei Golubchik wrote: Hi, Holyfoot! On Nov 30, holyf...@askmonty.org wrote: At file:///home/hf/wmar/mdev-5138/ revno: 3991 revision-id: holyf...@askmonty.org-20131130132432-etlmes8qhhqhr1iu parent: ele...@wheezy-64.home-20131128160251-vn857wx5qlon6j1p committer: Alexey Botchkov holyf...@askmonty.org branch nick: mdev-5138 timestamp: Sat 2013-11-30 17:24:32 +0400 message: MDEV-5138 Numerous test failures in mtr --ps --embedded. The function Protocol::net_store_data(a, b, CHARSET_A, CHARSET_B) should be adapted to be working in the embedded server as it's done with the Protocol::net_store_data(a, b). That new function renamed as net_store_data_cs, so we can make it virtual. Why does Protocol::net_store_data() have to be virtual? (if it doesn't, Protocol::net_store_data_cs doesn't have to be virtual either and doesn't have to be renamed) Regards, Sergei ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Re: [Maria-developers] [Commits] Rev 3991: MDEV-5138 Numerous test failures in mtr --ps --embedded
Hi, Holyfoot! On Nov 30, holyf...@askmonty.org wrote: At file:///home/hf/wmar/mdev-5138/ revno: 3991 revision-id: holyf...@askmonty.org-20131130132432-etlmes8qhhqhr1iu parent: ele...@wheezy-64.home-20131128160251-vn857wx5qlon6j1p committer: Alexey Botchkov holyf...@askmonty.org branch nick: mdev-5138 timestamp: Sat 2013-11-30 17:24:32 +0400 message: MDEV-5138 Numerous test failures in mtr --ps --embedded. The function Protocol::net_store_data(a, b, CHARSET_A, CHARSET_B) should be adapted to be working in the embedded server as it's done with the Protocol::net_store_data(a, b). That new function renamed as net_store_data_cs, so we can make it virtual. Why does Protocol::net_store_data() have to be virtual? (if it doesn't, Protocol::net_store_data_cs doesn't have to be virtual either and doesn't have to be renamed) Regards, Sergei ___ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
