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, <strings.h> 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<npoints;j++)
>> for(i=0;i<SPDIMS;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
