Hi Kristian, Thanks for the quick and thorough reply.
As to the patch, you're basically moving the check for NULL into all the > callers. I'm not sure if the check is even needed for all callers, but > then, > I also do not think it's worth it to try and determine where it is needed > and where not. > Looking at the code, I am about 90% certain that we do need the checks. Most of the code paths can execute with the pointer being null. > But wouldn't it be nicer to instead change the > give_error_if_slave_running() > to be a normal function that takes the Master_info_index pointer as an > argument (or a static method, if you want)? This way, you can keep the > null-pointer check in the function. Also helps avoid a new bug if another > caller is added. > The reason why I'm asking this about the return TRUE value is that it is something that Sergei wrote not too long ago (if 2 years can be considered not "too long" ago). I see how converting that to a function would be able to make us implement the guards better, but I feel that the semantics of it are best left as a member function. I would, however, wrap the null pointer check. Personally, I would've allocated this object as a global variable, adding the pesky LOCK code inside it and hiding it. I would have its members be pointers instead. That's a bit too much work for little gain however. I am inclined to push it _as is_ for now, but I'll wait for Sergei's opinion as well (he's on vacation right now, so it might take a bit). Thanks, Vicentiu
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
