Re: [Maria-developers] [Commits] 62021f3: MDEV-12070 - Introduce thd_query_safe() from MySQL 5.7
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/565d20b44f24fcc855dc616164d87b0...
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). Regards, Sergei Chief Architect MariaDB and security@mariadb.org
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/565d20b44f24fcc855dc616164d87b0...
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).
Hi Sergei, On Fri, Jun 23, 2017 at 12:47:46PM +0200, Sergei Golubchik wrote: 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
Hi, Sergey! On Jun 23, Sergey Vojtovich wrote:
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.
Makes sense. May be you can enforce it with an assert? At least, make sure that a function comment says that thd_query_string can only be used by the THD owner thread. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (2)
-
Sergei Golubchik
-
Sergey Vojtovich