List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:May 26 2011 1:07pm
Subject:Re: [Resend] bzr commit into mysql-trunk branch (tor.didriksen:3112)
Bug#57912
View as plain text  
Hello,

Thanks for making the effort needed to make this patch!

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));

CMake:
configure.cmake:CHECK_FUNCTION_EXISTS (bfill HAVE_BFILL)
configure.cmake:CHECK_FUNCTION_EXISTS (bzero HAVE_BZERO)

Comments:
include/m_string.h:/*  This is needed for the definitions of bzero... on 
solaris */
sql/hash_filo.h:#include "m_string.h"    /* bzero */
sql/sql_list.h:#include "m_string.h"                           /* bfill */
sql/table.cc:  free_root(&outparam->mem_root, MYF(0));       // Safe to 
call on bzero'd root
sql/sql_select.h:    All users do init_read_record(), which does bzero(),
sql/rpl_utility.h:#include "m_string.h"                           /* 
bzero, memcpy */
sql/debug_sync.cc:    Initialize action. Do not use bzero(). Strings may 
have malloced.
sql/sql_lex.cc:    I tried "bzero" in the sql_yacc.yy code, but that for
sql-common/client.c:  /* The rest of result members is bzeroed in malloc */
storage/myisam/mi_open.c:  /* prerequisites: bzero(info) && 
info->s=share; are met. */
storage/myisam/mi_cache.c:  is set) - unread part is bzero'ed
strings/ctype-mb.c:        just bfill using max_sort_char if 
max_sort_char is one byte.

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.

> @@ -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.

> === 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).

> === 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).

> === 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.

> === 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?

> === 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.

> === 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.

> === 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.

> === 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 -

> === 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.

> === 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.

> @@ -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.

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