maria-developers team mailing list archive

[Kristian Nielsen <knielsen@xxxxxxxxxxxxxxx>]

Alexi1952 <Alexi1952@xxxxxxxxx> writes:

> mysql_priv.h:

> (1)    void *sql_alloc(size_t);

> So I redefined sql_alloc() in a client context:
>
>         void *sql_alloc(size_t size) { ... }
>         ...
>         #include "sql_string.h"
>         #include "sql_list.h"

(Any reason you put the definition before the #include's ? Usually one writes
things the other way around)

> sql/sql_string.cc:
>
> (2)     extern uchar* sql_alloc(unsigned size);

> (3)     extern void sql_alloc(size_t size);

> - I found no direct usage of sql_alloc() in sql_string.* files;
> - After commenting (2) & (3) out the rebuild was successfull.
> If (2) & (3) shouldn't be deleted, then they should be brought

Seems like you should just delete them.

In any case extern declarations like this should be avoided, much better to
only have the declaration in one place in some *.h file.

> Looks like along with sql_alloc(), we can delete similar declarations
> (2a) & (3a) of sql_element_free() (and actually all occurences of this function):

> mysql_priv.h

> (1a)    void sql_element_free(void *ptr);

> thr_malloc.cc
>
>         void sql_element_free(void *ptr __attribute__((unused)))
>         {} /* purecov: deadcode */

> sql/sql_string.cc
>
> (2a)    extern void sql_element_free(void *ptr);
>
> client/sql_string.cc
>
> (3a)    extern void sql_element_free(void *ptr);
>
> Indeed, besides pointed above we find only the following
> occurences of sql_element_free():
>
> client/mysql.cc
>
>          void* sql_alloc(unsigned size);
>          void sql_element_free(void *ptr);
>          ...
>          void *sql_alloc(size_t Size)
>          {
>            return my_malloc(Size,MYF(MY_WME));
>          }
>          void sql_element_free(void *ptr)
>          {
>            my_free(ptr,MYF(0));
>          }
>
> So sql_element_free() is called nowhere (note also that this function
> has a do-nothing body).

Again I think the extern declarations should be deleted.

Not sure about the sql_element_free(). My guess would be that they were an
attempt to be able to use sql_list etc. from outside the server without memory
leaks, but the attempt was never completed. The issue with sql_alloc() is that
it uses a mem_root that will automatically free all memory at end of a
statement (for example). Maybe someone wanted to have explicit calls to
sql_element_free() (which would work in client but do nothing in server), but
they stopped half-way?

We might consider deleting them, on the other hand we should maybe better
leave them to reduce risk of conflicts when merging with MySQL. I have no
strong opinion on this one.

 - Kristian.