List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:May 26 2011 2:26pm
Subject:Re: [Resend] bzr commit into mysql-trunk branch (tor.didriksen:3112)
Bug#57912
View as plain text  
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(&reg,sizeof(reg));
>> +    memset(&reg,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

Thread
[Resend] bzr commit into mysql-trunk branch (tor.didriksen:3112) Bug#57912Tor Didriksen25 May
  • Re: [Resend] bzr commit into mysql-trunk branch (tor.didriksen:3112)Bug#57912Jon Olav Hauglid26 May
    • Re: [Resend] bzr commit into mysql-trunk branch (tor.didriksen:3112)Bug#57912Tor Didriksen26 May