maria-developers team mailing list archive

[Alexi1952 <Alexi1952@xxxxxxxxx>]

Thanks for your comments!

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

I agree. Currently, we have:

mysql_priv.h:

        #ifndef MYSQL_CLIENT
        ...
        void *sql_alloc(size_t);
        ...
        #endif

thr_malloc.cc:

        void *sql_alloc(size_t Size)
        {
          MEM_ROOT *root= *my_pthread_getspecific_ptr(MEM_ROOT**,THR_MALLOC);
          return alloc_root(root,Size);
        }

This states that sql_alloc() *should not* be used in MYSQL_CLIENT context
(hence all these extern declarations). In my opinion, it's more correctly to have:

mysql_priv.h:

        void *sql_alloc(size_t);

thr_malloc.cc:

        #ifndef MYSQL_CLIENT
        void *sql_alloc(size_t Size)
        {
          MEM_ROOT *root= *my_pthread_getspecific_ptr(MEM_ROOT**,THR_MALLOC);
          return alloc_root(root,Size);
        }
        #endif

which states instead that sql_alloc() *can be* used in a client context
but should be supplied with *another definition*.

Question: May I do these changes in mysql_priv.h and thr_malloc.cc ?
On the one hand, it is resonable. On the other hand, I am confused a bit
with "mysql_priv.h" name intended to mean that content of this file belongs
to mysql internals (though this file is included in mysqlbinlog.cc)

- Alex

09.10.09, 15:15, "Kristian Nielsen" <knielsen@xxxxxxxxxxxxxxx>:

> Alexi1952  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.
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp

-- 
Почта в порядке находится здесь: http://mail.yandex.ru/promo/new/order