maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

Hi Sergei,

On Fri, Jun 23, 2017 at 12:47:46PM +0200, Sergei Golubchik wrote:
> Hi, Sergey!
> 
> On Jun 22, Sergey Vojtovich wrote:
> > revision-id: 62021f391a42c5577190aa43cb8ad91e56235b46 (mariadb-10.2.6-57-g62021f3)
> > parent(s): 557e1bd472612848a42e772c1fb6f8ed32ab33b4
> > committer: Sergey Vojtovich
> > timestamp: 2017-06-22 17:15:10 +0400
> > message:
> > 
> > MDEV-12070 - Introduce thd_query_safe() from MySQL 5.7
> > 
> > Merged relevant part of MySQL revision:
> > https://github.com/mysql/mysql-server/commit/565d20b44f24fcc855dc616164d87b03cfad10bc
> > 
> > diff --git a/sql/sql_class.cc b/sql/sql_class.cc
> > index f8cf829..db65eb3 100644
> > --- a/sql/sql_class.cc
> > +++ b/sql/sql_class.cc
> > @@ -4529,6 +4529,32 @@ extern "C" LEX_STRING * thd_query_string (MYSQL_THD thd)
> >    return(&thd->query_string.string);
> >  }
> >  
> > +
> > +/**
> > +  Get the current query string for the thread.
> > +
> > +  @param thd     The MySQL internal thread pointer
> > +  @param buf     Buffer where the query string will be copied
> > +  @param buflen  Length of the buffer
> > +
> > +  @return Length of the query
> > +
> > +  @note This function is thread safe as the query string is
> > +        accessed under mutex protection and the string is copied
> > +        into the provided buffer. @see thd_query_string().
> > +*/
> > +
> > +extern "C" size_t thd_query_safe(MYSQL_THD thd, char *buf, size_t buflen)
> > +{
> > +  mysql_mutex_lock(&thd->LOCK_thd_data);
> > +  size_t len= MY_MIN(buflen - 1, thd->query_length());
> > +  memcpy(buf, thd->query(), len);
> > +  mysql_mutex_unlock(&thd->LOCK_thd_data);
> > +  buf[len]= '\0';
> > +  return len;
> > +}
> 
> Okay. But as neither thd_query_string() nor thd_query() are part of the
> plugin API, I'd suggest to remove them, they're inherently unsafe and
> should not be used. Few other engines use thd_query_string() need to be
> fixed too (but they bypass the plugin API and we don't promise stability
> for internal functions).
thd_query() - yes, this looks fairly broken.
thd_query_string() - I'd say this one should stay, because in most cases
we need to access query_string from the same thread; in this case we better
avoid mutex for performance reasons.

Thanks,
Sergey