maria-developers team mailing list archive

[Vicențiu Ciorbaru <vicentiu@xxxxxxxxxxx>]

Hi Sergei,

While working on features for 10.2 I've encountered a problem when
compiling MariaDB as a Debug build.

According to your change on
commit 29dd634a4c0b9c3579cf9d318ed64d748d848b1d
dbug: correct trace for DBUG_RETURN(func());

This causes a small problem, in two places within the codebase. One is in
the myisam storage engine and one in the maria storage engine.
Within thr_find_all_keys, there is a hack with goto statements and using
DBUG_ENTER after my_thread_init().

/Users/cvicentiu/Workspace/MariaDB/storage/myisam/sort.c:352:5: error:
cannot jump from this goto statement to its label
    goto err;
    ^
/Users/cvicentiu/Workspace/MariaDB/storage/myisam/sort.c:355:5: note: jump
bypasses initialization of variable with __attribute__((cleanup))
    DBUG_ENTER("thr_find_all_keys");
    ^
/Users/cvicentiu/Workspace/MariaDB/include/my_dbug.h:74:47: note: expanded
from macro 'DBUG_ENTER'
#define DBUG_ENTER(a) struct _db_stack_frame_ _db_stack_frame_
 __attribute__((cleanup(_db_return_))); \


In the current code state, the attribute_cleanup callback does not make
sense if my_thread_init() fails and forces the goto err; Technically, it is
undefined behaviour, as we're jumping within the block that has the
variables defined, without initialising it.

The sensible thing to do is to refactor the block that has DBUG_ENTER into
a separate function and decoupling the error cleanup from the
my_thread_init() call. I've looked at that code and have seen that much of
the logic is redundant during the cleanup part. I can refactor it to fix
the problem, but I'm looking for input on your side if you're ok with me
doing that.

I've also found a bug in that same code in the error checking that I can
fix.

Regards,
Vicentiu