oqgraph-dev team mailing list archive

[Andrew McDonnell <bugs@xxxxxxxxxxxxxxxxxxx>]

Hi Arjen

On 20/02/13 10:20, Arjen Lentz wrote:

<snipped>

>>
>> I am unfamiliar with the code as yet, so I don't know why this is
>> NULL.
>>
>> As an aside, I am used to working with code that is defensively laced
>> with assertions for NULL pointers, so this is a little different to me...
>
> Well that's clearly an omission and a bug in the new code.

If the bug is in the new code in passing in a bogus data structure, I guess I
was thinking, that table.cc seems to be in the core of MariaDB, and I was a
little surprised something central could be crashed by a broken plugin.

Anyway, would a patch to table.cc should go into the main code base in which
case, how would I go about that? Or should I just fix in my tree now and it
would get merged in the future with oqgraph?


> Crashing a multi-threaded/multi-user daemon is not nice. So generally when something fails in MySQL, you have a mechanism to return a fail from that function.
> At the very least you'd have an assert in there (do add it), but obviously you'd want to code to not get into that situation in the first place

Agreed on both :-)

init_tmp_table_share() is void however, so I am not sure what the usual
pattern is for MySQL/mariadb core code in these cases...

This function would appear to be part of the mysql API and perhaps should be
changed to return an error code if something unexpected happens - but who/how
decides these things?  For example, should it also check for key==NULL,
although this condition wont crash that function?

Another "odd" thing is this is C++ code with a bunch of classes, coexisting up
with a bunch of plain root namespace functions; and table.cc is 6900 lines
long... would it make sense to split this cc/h into multiple files one major
class per each, etc?


"Imposter syndrome" being I haven't done anything useful here yet and yet I am
asking such code-review type questions :-)


--Andrew