maria-developers team mailing list archive

[Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>]

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