Re: [Maria-developers] [Commits] Rev 3020: MWL#192: Non-blocking client API for libmysqlclient. in http://bazaar.launchpad.net/~maria-captains/maria/5.2
Hi Monty, Please find included the patch for MWL#192, non-blocking client API, for review. This patch is against MariaDB 5.2.8, though I expect the feature will eventually be merged and pushed to MariaDB 5.5. One thing in particular I would like to ask for advice on is if there are ways to improve the integration with libmysql/NET/Vio, as you know this code much better than me. - Kristian.
Hi Kristian, It is my own perception, but I feel the code got severely obfuscated through wrapper macros, and thus will be really hard to debug. Myself, I'd prefer it in a C/C++ source file, without macros (yes, it is means repetition, but I would not expect the result to be much larger in terms of lines of code with this). Also, the macros I talk about define global functions in a header file. At least on Windows it means linker error (multiple symbol definition) if the header is included into 2 different C files.
-----Original Message----- From: maria-developers- bounces+wlad=montyprogram.com@lists.launchpad.net [mailto:maria- developers-bounces+wlad=montyprogram.com@lists.launchpad.net] On Behalf Of Kristian Nielsen Sent: Dienstag, 20. September 2011 13:00 To: maria-developers@lists.launchpad.net; Michael Widenius Subject: Re: [Maria-developers] [Commits] Rev 3020: MWL#192: Non- blocking client API for libmysqlclient. in http://bazaar.launchpad.net/~maria- captains/maria/5.2
Hi Monty,
Please find included the patch for MWL#192, non-blocking client API, for review.
This patch is against MariaDB 5.2.8, though I expect the feature will eventually be merged and pushed to MariaDB 5.5.
One thing in particular I would like to ask for advice on is if there are ways to improve the integration with libmysql/NET/Vio, as you know this code much better than me.
- Kristian.
-----Original Message----- From: maria-developers- bounces+wlad=montyprogram.com@lists.launchpad.net [mailto:maria- developers-bounces+wlad=montyprogram.com@lists.launchpad.net] On Behalf Of Vladislav Vaintroub Sent: Dienstag, 20. September 2011 14:24 To: 'Kristian Nielsen'; maria-developers@lists.launchpad.net; 'Michael Widenius' Subject: Re: [Maria-developers] [Commits] Rev 3020: MWL#192: Non- blocking client API for libmysqlclient. in http://bazaar.launchpad.net/~maria- captains/maria/5.2
Hi Kristian, It is my own perception, but I feel the code got severely obfuscated
wrapper macros, and thus will be really hard to debug. Myself, I'd prefer it in a C/C++ source file, without macros (yes, it is means repetition, but I would not expect the result to be much larger in terms of lines of code with this). Also, the macros I talk about define global functions in a header file. At least on Windows it means linker error (multiple symbol definition) if
Hmm I missed something when reading this code. The wrapper macros are for internal testing use, for part of the API, they won't be debugged often, and not included into 2 different source files. Thus its usage seems ok now to me. Wlad through the
header is included into 2 different C files.
-----Original Message----- From: maria-developers- bounces+wlad=montyprogram.com@lists.launchpad.net [mailto:maria- developers-bounces+wlad=montyprogram.com@lists.launchpad.net] On Behalf Of Kristian Nielsen Sent: Dienstag, 20. September 2011 13:00 To: maria-developers@lists.launchpad.net; Michael Widenius Subject: Re: [Maria-developers] [Commits] Rev 3020: MWL#192: Non- blocking client API for libmysqlclient. in http://bazaar.launchpad.net/~maria- captains/maria/5.2
Hi Monty,
Please find included the patch for MWL#192, non-blocking client API, for review.
This patch is against MariaDB 5.2.8, though I expect the feature will eventually be merged and pushed to MariaDB 5.5.
One thing in particular I would like to ask for advice on is if there are ways to improve the integration with libmysql/NET/Vio, as you know this code much better than me.
- Kristian.
_______________________________________________ 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
"Vladislav Vaintroub" <wlad@montyprogram.com> writes:
Hmm I missed something when reading this code. The wrapper macros are for internal testing use, for part of the API, they won't be debugged often, and not included into 2 different source files. Thus its usage seems ok now to me.
Ah ok, I'll leave them then. Still, thanks for checking! - Kristian.
"Vladislav Vaintroub" <wlad@montyprogram.com> writes:
It is my own perception, but I feel the code got severely obfuscated through wrapper macros, and thus will be really hard to debug. Myself, I'd prefer it in a C/C++ source file, without macros (yes, it is means repetition, but I would not expect the result to be much larger in terms of lines of code with this).
Are you sure? It seems to me a nightmare to maintain the code if the macros were expanded? Imagine making a change to the wrapper code. Then such change would need to be made manually in 40 (or whatever) function definitions. It seems this would very easily lead to subtle errors where one function was incorrectly edited and so on ... I mean, sure C macro magic is to be minimised, I agree, but I don't see how? Or if do you have a concrete suggestion/patch on how to do it better, I'd like to see it, a version without the macros and without the update/maintenance problem would indeed be better.
Also, the macros I talk about define global functions in a header file. At least on Windows it means linker error (multiple symbol definition) if the header is included into 2 different C files.
Here I believe you are talking about nonblock-wrappers.h? Note that this is only used for test cases in mysqltest.cc and mysql_client_test.c, it shouldn't be used anywhere else and is not installed anywhere. So there should be no risk of including multiple times. This #include could be avoided by instead introducing a .c file and compiling+linking that in both mysqltest and mysql_client_test. Do you think that would be better? Or we could even make a libnonblock_wrapper with just this code, though that seems unnecessarily complex to me, what do you think? Thanks, - Kristian.
participants (2)
-
Kristian Nielsen
-
Vladislav Vaintroub