On February 20, 2019, Jan Lindström wrote:
revision-id: dadf5f78411b8f46ac53b8ea12c08ec317d9aab4 (mariadb-10.1.38-7-gdadf5f78411) parent(s): 346e46089621e6951e076c82ed5690aa23dcb5fe author: Jan Lindström committer: Jan Lindström timestamp: 2019-02-20 08:10:26 +0200 message:
Fix few compiler warnings.
The commit message should include the ticket number. I would also suggest fixing the grammar: MDEV-18659: Fix a few compiler warnings Could you make the title more specific?
dict_table_rename_in_cache dict_create_add_foreign_id Use of strncpy caused string truncation warnings and is replaced with memcpy.
Specifically, which warnings were issued? I am concerned that this could be replacing dubious code with worse. With memcpy(), you should be sure that the source strings is really safe to access up to the specified length. With strncpy(), the copying will stop at the first NUL character in the output, or at the specified length, whichever comes first.
fts_fetch_index_words dfield_get_len() can return UNIV_SQL_NULL (=ULINT32_UNDEFINED) that is too big for used short datatype. Replaced with correct ulint.
Is NULL really an allowed value there? I do not think that the "words" or "tokens" (the keys of the inverted index) should ever be NULL. Likewise, if the entire fulltext-indexed column is NULL, we should not be inserting anything into the inverted index. Please ensure that these cases are covered by test cases in mysql-test-run, and share the detailed results: especially, which pieces of code are handling the NULL values. The data that is being fed to fts_fetch_index_words() comes from this query to the InnoDB internal SQL parser: " SELECT word\n" " FROM $table_name\n" " WHERE word > :word\n" " ORDER BY word;\n" These tables appear to be created in fts_create_one_index_table(), and it seems to me that it is PRIMARY KEY(word,first_doc_id). Strangely, I do not see DATA_NOT_NULL in the column creation, so it is possible that we are reserving NULL flags for the "word" column. But, InnoDB is not prepared to deal with NULL values in the primary key. Lots of things would break if "word" were NULL. Please add assertions to the places where "word" is being written or read in these tables. Here is the call that forgets to declare "word" as NOT NULL: dict_mem_table_add_col(new_table, heap, "word", charset == &my_charset_latin1 ? DATA_VARCHAR : DATA_VARMYSQL, field->col->prtype, FTS_MAX_WORD_LEN_IN_CHAR * unsigned(field->col->mbmaxlen));
row_rename_table_for_mysql Target string was exactly same length as maximum length of copied string.
Fixing this does make sense.
diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 06c6c3effab..04b14e3ae06 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -1845,7 +1845,7 @@ dict_table_rename_in_cache(
ulint db_len; char* old_id; - char old_name_cs_filename[MAX_TABLE_NAME_LEN+20]; + char old_name_cs_filename[MAX_TABLE_NAME_LEN+20]=""; uint errors = 0;
/* All table names are internally stored in charset
What is the initialization needed for? It was not mentioned in the commit comment. Note that this would likely not only initialize the first byte of the string, but all bytes. What kind of machine code is this generating? memcpy()? Or is the compiler smart enough to recognize that the stack variable is going to be zero-filled, and will emit something like memset() instead? Either way, it is not good to write MAX_TABLE_NAME_LEN+20 bytes if only 1 byte (a terminating NUL character at the start of the array) would suffice. And you would have to convince me that this initialization is necessary in the first place. Best regards, Marko Mäkelä Lead Developer InnoDB MariaDB Corporation