From: Tor Didriksen Date: May 26 2011 2:26pm Subject: Re: [Resend] bzr commit into mysql-trunk branch (tor.didriksen:3112) Bug#57912 List-Archive: http://lists.mysql.com/commits/138221 Message-Id: <4DDE631D.6070301@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 2011-05-26 15:07, Jon Olav Hauglid wrote: > Hello, > > Thanks for making the effort needed to make this patch! ditto for reviewing > > On 05/25/2011 03:33 PM, Tor Didriksen wrote: >> 3112 Tor Didriksen 2011-05-25 >> Bug #57912 Use memset() rather than the deprecated bfill() and >> bzero() > > Use Bug#11765003 instead of old number. > > Consider sending an e-mail to dev-private after this patch > has been pushed informing developers what will happen if > they continue to use bzero/bfill on trunk. > > I found some remaining uses of bzero/bfill: > > Code: > strings/uctypedump.c: bzero(&ch, sizeof(ch)); > strings/uctypedump.c: bzero(unidata, sizeof(unidata)); > strings/uca-dump.c: bzero(weight, weight_elements * sizeof(*weight)); > strings/uca-dump.c: bzero(&uca, sizeof(uca)); > strings/uca-dump.c: bzero(pageloaded, sizeof(pageloaded)); oops, files which did not #include m_string.h I guess. actually no, files which are not compiled ... There's a target for uctypedump in 5.1, but it does not compile ... > > CMake: > configure.cmake:CHECK_FUNCTION_EXISTS (bfill HAVE_BFILL) > configure.cmake:CHECK_FUNCTION_EXISTS (bzero HAVE_BZERO) > gone, ditto in config.h.cmake and WindowsCache.cmake > Comments: > include/m_string.h:/* This is needed for the definitions of bzero... > on solaris */ that can go away, is already included.... > sql/hash_filo.h:#include "m_string.h" /* bzero */ don't dare to remove #include, removed comment > sql/sql_list.h:#include "m_string.h" /* > bfill */ don't dare to remove #include, removed comment > sql/table.cc: free_root(&outparam->mem_root, MYF(0)); // Safe > to call on bzero'd root hmm, removed the comment > sql/sql_select.h: All users do init_read_record(), which does bzero(), fixed. > sql/rpl_utility.h:#include "m_string.h" /* > bzero, memcpy */ don't dare to remove #include, removed comment > sql/debug_sync.cc: Initialize action. Do not use bzero(). Strings > may have malloced. changed comment > sql/sql_lex.cc: I tried "bzero" in the sql_yacc.yy code, but that for I cannot grok that comment: "I tried bzero, but that made the values zero" ... I prefer to leave it alone. > sql-common/client.c: /* The rest of result members is bzeroed in > malloc */ fixed comment > storage/myisam/mi_open.c: /* prerequisites: bzero(info) && > info->s=share; are met. */ fixed comment > storage/myisam/mi_cache.c: is set) - unread part is bzero'ed fixed comment > strings/ctype-mb.c: just bfill using max_sort_char if > max_sort_char is one byte. fixed comment > > I don't think all comments need to be changed, but could you > take a look? > > I also have a few code comments, mostly code style nitpicks: > >> === modified file 'client/mysqltest.cc' >> --- a/client/mysqltest.cc 2011-05-05 23:52:44 +0000 >> +++ b/client/mysqltest.cc 2011-05-25 13:16:11 +0000 >> @@ -9264,7 +9264,7 @@ struct st_replace_regex* init_replace_re >> /* for each regexp substitution statement */ >> while (p< expr_end) >> { >> - bzero(®,sizeof(reg)); >> + memset(®,0, sizeof(reg)); > > Space after comma. OK > >> @@ -9717,7 +9717,7 @@ REPLACE *init_replace(char * *from, char >> if (len> max_length) >> max_length=len; >> } >> - bzero((char*) is_word_end,sizeof(is_word_end)); >> + memset(is_word_end,0, sizeof(is_word_end)); > > Space after comma. OK > >> === modified file 'libmysql/libmysql.c' >> --- a/libmysql/libmysql.c 2011-05-06 13:46:57 +0000 >> +++ b/libmysql/libmysql.c 2011-05-25 13:16:11 +0000 >> @@ -4417,7 +4417,7 @@ int STDCALL mysql_stmt_store_result(MYSQ >> */ >> MYSQL_BIND *my_bind, *end; >> MYSQL_FIELD *field; >> - bzero((char*) stmt->bind, sizeof(*stmt->bind)* stmt->field_count); >> + memset(stmt->bind, 0, sizeof(*stmt->bind)* stmt->field_count); > > Inconsistent spacing around * (multiplication). OK > >> === modified file 'sql/records.cc' >> --- a/sql/records.cc 2011-03-22 11:44:40 +0000 >> +++ b/sql/records.cc 2011-05-25 13:16:11 +0000 >> @@ -590,7 +590,8 @@ static int init_rr_cache(THD *thd, READ_ >> DBUG_RETURN(1); >> #ifdef HAVE_purify >> // Avoid warnings in qsort >> - bzero(info->cache,rec_cache_size+info->cache_records* >> info->struct_length+1); >> + memset(info->cache, 0, >> + rec_cache_size+info->cache_records* info->struct_length+1); > > Inconsistent spacing around * (multiplication). OK (also space around '+') > >> === modified file 'sql/sql_select.cc' >> --- a/sql/sql_select.cc 2011-05-18 10:43:46 +0000 >> +++ b/sql/sql_select.cc 2011-05-25 13:16:11 +0000 >> @@ -16302,8 +16302,8 @@ TABLE *create_duplicate_weedout_tmp_tabl >> >> >> /* STEP 4: Create TABLE description */ >> - bzero((char*) table,sizeof(*table)); >> - bzero((char*) reg_field,sizeof(Field*)*2); >> + memset(table, 0, sizeof(*table)); >> + memset(reg_field,0, sizeof(Field*)*2); > > Space after comma. OK > >> === modified file 'sql/sql_parse.cc' >> --- a/sql/sql_parse.cc 2011-05-21 08:25:33 +0000 >> +++ b/sql/sql_parse.cc 2011-05-25 13:16:11 +0000 >> @@ -5545,7 +5545,7 @@ void create_select_for_variable(const ch >> lex->sql_command= SQLCOM_SELECT; >> tmp.str= (char*) var_name; >> tmp.length=strlen(var_name); >> - bzero((char*) &null_lex_string.str, sizeof(null_lex_string)); >> + memset(&null_lex_string.str, 0, sizeof(null_lex_string)); > > Isn't this code a little suspect? :-) str is at offset zero, so it works, but OK: changed it > >> === modified file 'sql/table.cc' >> --- a/sql/table.cc 2011-05-23 23:34:47 +0000 >> +++ b/sql/table.cc 2011-05-25 13:16:11 +0000 >> @@ -1731,8 +1731,8 @@ static int open_binary_frm(THD *thd, TAB >> { >> /* Old file format with default as not null */ >> uint null_length= (share->null_fields+7)/8; >> - bfill(share->default_values + (null_flags - (uchar*) record), >> - null_length, 255); >> + memset(share->default_values + (null_flags - (uchar*) record), 255, > > Trailing space. OK > >> === modified file 'storage/heap/hp_test2.c' >> --- a/storage/heap/hp_test2.c 2011-01-11 09:09:21 +0000 >> +++ b/storage/heap/hp_test2.c 2011-05-25 13:16:11 +0000 >> @@ -63,8 +63,8 @@ int main(int argc, char *argv[]) >> filename2= "test2_2"; >> file=file2=0; >> get_options(argc,argv); >> - >> - bzero(&hp_create_info, sizeof(hp_create_info)); >> + >> + memset(&hp_create_info, 0, sizeof(hp_create_info)); > > Line with just spaces. OK > >> === modified file 'storage/innobase/handler/ha_innodb.cc' >> --- a/storage/innobase/handler/ha_innodb.cc 2011-05-22 20:17:42 +0000 >> +++ b/storage/innobase/handler/ha_innodb.cc 2011-05-25 13:16:11 +0000 > > Probably should give #innodb a heads-up about changing one of their > files. > OK >> === modified file 'storage/myisam/mi_check.c' >> --- a/storage/myisam/mi_check.c 2011-03-29 12:56:34 +0000 >> +++ b/storage/myisam/mi_check.c 2011-05-25 13:16:11 +0000 >> @@ -4044,8 +4044,8 @@ static int sort_insert_key(MI_SORT_PARAM >> >> /* Fill block with end-zero and write filled block */ >> mi_putint(anc_buff,key_block->last_length,nod_flag); >> - bzero((uchar*) anc_buff+key_block->last_length, >> - keyinfo->block_length- key_block->last_length); >> + memset(anc_buff+key_block->last_length, 0, >> + keyinfo->block_length- key_block->last_length); > > Inconsistent spacing around - OK > >> === modified file 'storage/myisam/mi_open.c' >> --- a/storage/myisam/mi_open.c 2011-01-11 09:09:21 +0000 >> +++ b/storage/myisam/mi_open.c 2011-05-25 13:16:11 +0000 >> @@ -1011,7 +1011,7 @@ uint mi_base_info_write(File file, MI_BA >> mi_int2store(ptr,base->max_key_length); ptr +=2; >> mi_int2store(ptr,base->extra_alloc_bytes); ptr +=2; >> *ptr++= base->extra_alloc_procent; >> - bzero(ptr,13); ptr +=13; /* extra */ >> + memset(ptr,0 , 13); ptr +=13; /* extra */ > > Space after comma. OK > >> === modified file 'storage/myisam/sp_test.c' >> --- a/storage/myisam/sp_test.c 2011-03-28 08:49:43 +0000 >> +++ b/storage/myisam/sp_test.c 2011-05-25 13:16:11 +0000 >> @@ -332,8 +332,8 @@ static void create_linestring(uchar *rec >> for(j=0;j> for(i=0;i> x[i+j*SPDIMS]=rownr*j; >> - >> - bzero((char*) record,MAX_REC_LENGTH); >> + >> + memset(record, 0, MAX_REC_LENGTH); > > Line with just spaces. OK > >> @@ -353,8 +353,8 @@ static void create_key(uchar *key,uint r >> double c=rownr; >> uchar *pos; >> uint i; >> - >> - bzero(key,MAX_REC_LENGTH); >> + >> + memset(key, 0, MAX_REC_LENGTH); > > Line with just spaces. OK > > Thanks, > > --- Jon Olav