Re: [Maria-developers] [Commits] Rev 3100: Fix gcc 4.6 warnings about assigned but not used variables. in file:///home/bell/maria/bzr/work-maria-5.1-applebuild/
Hi!
"sanja" == sanja <sanja@askmonty.org> writes:
sanja> At file:///home/bell/maria/bzr/work-maria-5.1-applebuild/ sanja> ------------------------------------------------------------ sanja> revno: 3100 sanja> revision-id: sanja@askmonty.org-20111025142424-dntpnipgi5qxg2io sanja> parent: wlad@montyprogram.com-20111019185101-fw3oxkpr2o060qk4 sanja> committer: sanja@askmonty.org sanja> branch nick: work-maria-5.1-applebuild sanja> timestamp: Tue 2011-10-25 17:24:24 +0300 sanja> message: sanja> Fix gcc 4.6 warnings about assigned but not used variables. sanja> Fixed my_gethwaddr.c to allow compilation on Mac OS X. <cut> === modified file 'sql/sql_show.cc' --- a/sql/sql_show.cc 2011-05-12 11:56:08 +0000 +++ b/sql/sql_show.cc 2011-10-25 14:24:24 +0000 @@ -1215,7 +1215,7 @@ int store_create_info(THD *thd, TABLE_LI handler *file= table->file; TABLE_SHARE *share= table->s; HA_CREATE_INFO create_info; - bool show_table_options= FALSE; + bool show_table_options __attribute__ ((unused))= FALSE; bool foreign_db_mode= (thd->variables.sql_mode & (MODE_POSTGRESQL | MODE_ORACLE | MODE_MSSQL | Add instead: #ifdef WITH_PARTITION_STORAGE_ENGINE around the this the definition of the variable. @@ -1432,7 +1432,9 @@ int store_create_info(THD *thd, TABLE_LI packet->append(STRING_WITH_LEN("\n)")); if (!(thd->variables.sql_mode & MODE_NO_TABLE_OPTIONS) && !foreign_db_mode) { +#ifdef WITH_PARTITION_STORAGE_ENGINE show_table_options= TRUE; +#endif The above is correct; Better to do it this way. === modified file 'storage/maria/ma_blockrec.c' --- a/storage/maria/ma_blockrec.c 2011-07-24 08:25:28 +0000 +++ b/storage/maria/ma_blockrec.c 2011-10-25 14:24:24 +0000 @@ -4997,7 +4997,7 @@ static my_bool read_row_extent_info(MARI MARIA_EXTENT_CURSOR extent; MARIA_RECORD_POS *tail_pos; uchar *data, *end_of_data; - uint flag, row_extents, row_extents_size, field_lengths; + uint flag, row_extents, row_extents_size; uchar *extents, *end; DBUG_ENTER("read_row_extent_info"); @@ -5032,9 +5032,6 @@ static my_bool read_row_extent_info(MARI } info->cur_row.extents_count= row_extents; - if (share->base.max_field_lengths) - get_key_length(field_lengths, data); - The above is wrong. get_key_length will increment data, which is required as data it's used later. Better to leave field_lengths there for the future. (Please fix this in 5.3 too !) You can avoid the warning by doing: get_key_length= get_key_length; === modified file 'storage/maria/ma_dynrec.c' --- a/storage/maria/ma_dynrec.c 2011-02-25 12:55:40 +0000 +++ b/storage/maria/ma_dynrec.c 2011-10-25 14:24:24 +0000 @@ -1741,7 +1741,8 @@ int _ma_read_rnd_dynamic_record(MARIA_HA MARIA_RECORD_POS filepos, my_bool skip_deleted_blocks) { - int block_of_record, info_read; + int block_of_record; + int info_read; Why the above? === modified file 'storage/maria/ma_ft_update.c' --- a/storage/maria/ma_ft_update.c 2009-01-30 21:55:42 +0000 +++ b/storage/maria/ma_ft_update.c 2011-10-25 14:24:24 +0000 @@ -319,7 +319,6 @@ my_bool _ma_ft_convert_to_ft2(MARIA_HA * uchar *key_ptr= (uchar*) dynamic_array_ptr(da, 0), *end; uint length, key_length; MARIA_PINNED_PAGE tmp_page_link, *page_link= &tmp_page_link; - MARIA_KEY tmp_key; MARIA_PAGE page; DBUG_ENTER("_ma_ft_convert_to_ft2"); @@ -357,13 +356,8 @@ my_bool _ma_ft_convert_to_ft2(MARIA_HA * /* inserting the rest of key values */ end= (uchar*) dynamic_array_ptr(da, da->elements); - tmp_key.keyinfo= keyinfo; - tmp_key.data_length= keyinfo->keylength; - tmp_key.ref_length= 0; - tmp_key.flag= 0; for (key_ptr+=length; key_ptr < end; key_ptr+=keyinfo->keylength) { - tmp_key.data= key_ptr; if (_ma_ck_real_write_btree(info, key, &root, SEARCH_SAME)) DBUG_RETURN(1); } Wrong. The correct fix is: if (_ma_ck_real_write_btree(info, key, &root, SEARCH_SAME)) DBUG_RETURN(1); -> if (_ma_ck_real_write_btree(info, &tmp_key, &root, SEARCH_SAME)) DBUG_RETURN(1); Apparently free textsearch with Aria is not tested properly :( <cut> === modified file 'storage/maria/ma_open.c' --- a/storage/maria/ma_open.c 2011-07-24 08:25:28 +0000 +++ b/storage/maria/ma_open.c 2011-10-25 14:24:24 +0000 @@ -1802,7 +1802,6 @@ void _ma_set_index_pagecache_callbacks(P int _ma_open_datafile(MARIA_HA *info, MARIA_SHARE *share, const char *org_name, File file_to_dup __attribute__((unused))) { - char *data_name= share->data_file_name.str; char real_data_name[FN_REFLEN]; if (org_name) @@ -1816,7 +1815,6 @@ int _ma_open_datafile(MARIA_HA *info, MA my_errno= HA_WRONG_CREATE_OPTION; return 1; } - data_name= real_data_name; } } Wrong: The correct fix is: info->dfile.file= share->bitmap.file.file= my_open(share->data_file_name.str, share->mode | O_SHARE, MYF(MY_WME)); -> info->dfile.file= share->bitmap.file.file= my_open(data_name, share->mode | O_SHARE, MYF(MY_WME)); The above means that symlinking didn't work on windows with Aria files. === modified file 'storage/maria/ma_rt_split.c' --- a/storage/maria/ma_rt_split.c 2010-09-05 23:25:44 +0000 +++ b/storage/maria/ma_rt_split.c 2011-10-25 14:24:24 +0000 @@ -380,7 +380,6 @@ int maria_rtree_split_page(const MARIA_K SplitStruct *stop; double *coord_buf; double *next_coord; - double *old_coord; int n_dim; uchar *source_cur, *cur1, *cur2; uchar *new_page_buff, *log_internal_copy, *log_internal_copy_ptr, @@ -426,7 +425,6 @@ int maria_rtree_split_page(const MARIA_K maria_rtree_d_mbr(keyinfo->seg, key->data, key_data_length, cur->coords); cur->key= key->data; - old_coord= next_coord; myisam/rt_split.c has the exact same code. You should do the same change there. === modified file 'storage/myisam/mi_check.c' --- a/storage/myisam/mi_check.c 2011-05-02 17:58:45 +0000 +++ b/storage/myisam/mi_check.c 2011-10-25 14:24:24 +0000 @@ -946,7 +946,6 @@ int chk_data_link(HA_CHECK *param, MI_IN char llbuff[22],llbuff2[22],llbuff3[22]; ha_checksum intern_record_checksum; ha_checksum key_checksum[HA_MAX_POSSIBLE_KEY]; - my_bool static_row_size; MI_KEYDEF *keyinfo; MI_BLOCK_INFO block_info; DBUG_ENTER("chk_data_link"); @@ -971,7 +970,6 @@ int chk_data_link(HA_CHECK *param, MI_IN empty=info->s->pack.header_length; /* Check how to calculate checksum of rows */ - static_row_size=1; if (info->s->data_file_type == COMPRESSED_RECORD) { for (field=0 ; field < info->s->base.fields ; field++) @@ -979,7 +977,6 @@ int chk_data_link(HA_CHECK *param, MI_IN if (info->s->rec[field].base_type == FIELD_BLOB || info->s->rec[field].base_type == FIELD_VARCHAR) { - static_row_size=0; break; } } Please remove also the if and the loop around the above code. === modified file 'storage/pbxt/src/ha_pbxt.cc' --- a/storage/pbxt/src/ha_pbxt.cc 2010-11-26 22:37:34 +0000 +++ b/storage/pbxt/src/ha_pbxt.cc 2011-10-25 14:24:24 +0000 @@ -1559,7 +1559,7 @@ static int pbxt_rollback(handlerton *hto if (!all) self->st_stat_trans = FALSE; } - return 0; + return err; } #ifdef DRIZZLED @@ -2863,7 +2863,7 @@ int ha_pbxt::update_row(const byte * old * insert into t1 (val) values (1); */ if (table->found_next_number_field && new_data == table->record[0]) { - MX_LONGLONG_T nr; + MX_LONGLONG_T nr __attribute__ ((unused)); my_bitmap_map *old_map; old_map = mx_tmp_use_all_columns(table, table->read_set); Better to remove the nr value totally (and the call that sets it). This is safe as in the code: nr = table->found_next_number_field->val_int(); ha_set_auto_increment(pb_open_tab, table->found_next_number_field); The first thing ha_set_auto_increment() is doing is calling val_int() on the second argument. <cut> === modified file 'storage/pbxt/src/thread_xt.cc' --- a/storage/pbxt/src/thread_xt.cc 2010-09-28 13:05:45 +0000 +++ b/storage/pbxt/src/thread_xt.cc 2011-10-25 14:24:24 +0000 @@ -485,7 +485,8 @@ static void thr_free_resources(XTThreadP xtPublic void xt_bug(XTThreadPtr XT_UNUSED(self)) { - static int *bug_ptr = NULL; + static int *bug_ptr __attribute__ ((unused)); + bug_ptr= NULL; bug_ptr = NULL; } No reason to set this to null twice. Better to remove the variable altogether. === modified file 'storage/pbxt/src/xactlog_xt.cc' --- a/storage/pbxt/src/xactlog_xt.cc 2010-05-05 10:59:57 +0000 +++ b/storage/pbxt/src/xactlog_xt.cc 2011-10-25 14:24:24 +0000 @@ -2117,7 +2117,7 @@ xtBool XTDatabaseLog::xlog_seq_next(XTXa size_t rec_offset; size_t max_rec_len; size_t size; - u_int check_size = 1; + u_int check_size __attribute__ ((unused))= 1; Don't understand the above change. We both set check_size and test it in the above function. Are you sure that you get a warning about the above? /* Go to the next record (xseq_record_len must be initialized * to 0 for this to work. @@ -2629,7 +2629,7 @@ static void *xlog_wr_run_thread(XTThread { XTDatabaseHPtr db = (XTDatabaseHPtr) self->t_data; int count; - void *mysql_thread; + void *mysql_thread __attribute__ ((unused)); mysql_thread = myxt_create_thread(); === modified file 'storage/xtradb/btr/btr0cur.c' --- a/storage/xtradb/btr/btr0cur.c 2011-04-29 14:16:42 +0000 +++ b/storage/xtradb/btr/btr0cur.c 2011-10-25 14:24:24 +0000 @@ -3301,7 +3301,7 @@ static void btr_record_not_null_field_in_rec( /*=============================*/ - rec_t* rec, /*!< in: physical record */ + rec_t* rec __attribute__ ((unused)),/*!< in: physical record */ ulint n_unique, /*!< in: dict_index_get_n_unique(index), number of columns uniquely determine an index entry */ @@ -3321,9 +3321,8 @@ btr_record_not_null_field_in_rec( for (i = 0; i < n_unique; i++) { ulint rec_len; - byte* field; - field = rec_get_nth_field(rec, offsets, i, &rec_len); + rec_get_nth_field_offs(offsets, i, &rec_len); if (rec_len != UNIV_SQL_NULL) { n_not_null[i]++; === modified file 'storage/xtradb/buf/buf0buf.c' --- a/storage/xtradb/buf/buf0buf.c 2011-04-29 14:16:42 +0000 +++ b/storage/xtradb/buf/buf0buf.c 2011-10-25 14:24:24 +0000 @@ -3895,7 +3895,7 @@ buf_page_io_complete( enum buf_io_fix io_type; const ibool uncompressed = (buf_page_get_state(bpage) == BUF_BLOCK_FILE_PAGE); - enum buf_flush flush_type; + //enum buf_flush flush_type; mutex_t* block_mutex; ut_a(buf_page_in_file(bpage)); @@ -4051,7 +4051,7 @@ corrupt: //buf_pool_mutex_enter(); if (io_type == BUF_IO_WRITE) { - flush_type = buf_page_get_flush_type(bpage); + //flush_type = buf_page_get_flush_type(bpage); /* to keep consistency at buf_LRU_insert_zip_clean() */ //if (flush_type == BUF_FLUSH_LRU) { /* optimistic! */ mutex_enter(&LRU_list_mutex); Remove the variable from the start of the function and enum buf_flush flush_type; to the if. That way, all the comments are in one place. Ok to push after the above changes. Regards, Monty
26.10.2011 22:18, Michael Widenius пишет:
Hi!
"sanja" == sanja<sanja@askmonty.org> writes: sanja> At file:///home/bell/maria/bzr/work-maria-5.1-applebuild/ sanja> ------------------------------------------------------------ sanja> revno: 3100 sanja> revision-id: sanja@askmonty.org-20111025142424-dntpnipgi5qxg2io sanja> parent: wlad@montyprogram.com-20111019185101-fw3oxkpr2o060qk4 sanja> committer: sanja@askmonty.org sanja> branch nick: work-maria-5.1-applebuild sanja> timestamp: Tue 2011-10-25 17:24:24 +0300 sanja> message: sanja> Fix gcc 4.6 warnings about assigned but not used variables.
sanja> Fixed my_gethwaddr.c to allow compilation on Mac OS X.
<cut>
=== modified file 'sql/sql_show.cc' --- a/sql/sql_show.cc 2011-05-12 11:56:08 +0000 +++ b/sql/sql_show.cc 2011-10-25 14:24:24 +0000 @@ -1215,7 +1215,7 @@ int store_create_info(THD *thd, TABLE_LI handler *file= table->file; TABLE_SHARE *share= table->s; HA_CREATE_INFO create_info; - bool show_table_options= FALSE; + bool show_table_options __attribute__ ((unused))= FALSE; bool foreign_db_mode= (thd->variables.sql_mode& (MODE_POSTGRESQL | MODE_ORACLE | MODE_MSSQL |
Add instead:
#ifdef WITH_PARTITION_STORAGE_ENGINE
around the this the definition of the variable.
This is what previous reviewer was against. And I think it was quite logical. Please settle it with serg. [skip]
26.10.2011 22:18, Michael Widenius пишет: [skip]
=== modified file 'storage/pbxt/src/thread_xt.cc' --- a/storage/pbxt/src/thread_xt.cc 2010-09-28 13:05:45 +0000 +++ b/storage/pbxt/src/thread_xt.cc 2011-10-25 14:24:24 +0000 @@ -485,7 +485,8 @@ static void thr_free_resources(XTThreadP
xtPublic void xt_bug(XTThreadPtr XT_UNUSED(self)) { - static int *bug_ptr = NULL; + static int *bug_ptr __attribute__ ((unused)); + bug_ptr= NULL;
bug_ptr = NULL; }
No reason to set this to null twice. Better to remove the variable altogether.
Th code was super strange before, I left it strange :) I think it is better to Percona to decide what to do with it (I sent them patch for 5.3). [skip]
Oleksandr Byelkin <sanja@askmonty.org> writes:
26.10.2011 22:18, Michael Widenius пишет: [skip]
=== modified file 'storage/pbxt/src/thread_xt.cc' --- a/storage/pbxt/src/thread_xt.cc 2010-09-28 13:05:45 +0000 +++ b/storage/pbxt/src/thread_xt.cc 2011-10-25 14:24:24 +0000 @@ -485,7 +485,8 @@ static void thr_free_resources(XTThreadP
xtPublic void xt_bug(XTThreadPtr XT_UNUSED(self)) { - static int *bug_ptr = NULL; + static int *bug_ptr __attribute__ ((unused)); + bug_ptr= NULL;
bug_ptr = NULL; }
Th code was super strange before, I left it strange :) I think it is better to Percona to decide what to do with it (I sent them patch for 5.3).
Did you really send to Percona, or did you mean PBXT? Either way, PBXT it should be :) - Kristian.
27.10.2011 15:29, Kristian Nielsen пишет:
Oleksandr Byelkin<sanja@askmonty.org> writes:
26.10.2011 22:18, Michael Widenius пишет: [skip]
=== modified file 'storage/pbxt/src/thread_xt.cc' --- a/storage/pbxt/src/thread_xt.cc 2010-09-28 13:05:45 +0000 +++ b/storage/pbxt/src/thread_xt.cc 2011-10-25 14:24:24 +0000 @@ -485,7 +485,8 @@ static void thr_free_resources(XTThreadP
xtPublic void xt_bug(XTThreadPtr XT_UNUSED(self)) { - static int *bug_ptr = NULL; + static int *bug_ptr __attribute__ ((unused)); + bug_ptr= NULL;
bug_ptr = NULL; } Th code was super strange before, I left it strange :) I think it is better to Percona to decide what to do with it (I sent them patch for 5.3). Did you really send to Percona, or did you mean PBXT? Either way, PBXT it should be :) Actually I sent whole patch to percona hoping they will take part they want.
About pbxt - I was informed that nobody care about it, so did not do special movement... In any case the patch is published and code was preserved as it was :)
Oleksandr Byelkin <sanja@askmonty.org> writes:
27.10.2011 15:29, Kristian Nielsen пишет:
Oleksandr Byelkin<sanja@askmonty.org> writes:
26.10.2011 22:18, Michael Widenius пишет: [skip]
=== modified file 'storage/pbxt/src/thread_xt.cc' --- a/storage/pbxt/src/thread_xt.cc 2010-09-28 13:05:45 +0000 +++ b/storage/pbxt/src/thread_xt.cc 2011-10-25 14:24:24 +0000 @@ -485,7 +485,8 @@ static void thr_free_resources(XTThreadP
xtPublic void xt_bug(XTThreadPtr XT_UNUSED(self)) { - static int *bug_ptr = NULL; + static int *bug_ptr __attribute__ ((unused)); + bug_ptr= NULL;
bug_ptr = NULL; } Th code was super strange before, I left it strange :) I think it is better to Percona to decide what to do with it (I sent them patch for 5.3). Did you really send to Percona, or did you mean PBXT? Either way, PBXT it should be :) Actually I sent whole patch to percona hoping they will take part they want.
About pbxt - I was informed that nobody care about it, so did not do special movement... In any case the patch is published and code was preserved as it was :)
Ok, I misunderstood then. I thought you meant that you sent the above patch to thread_xt.cc to Percona, I just wanted to point out that this is PBXT code, not Percona code. Sorry for any confusion. - Kristian.
participants (3)
-
Kristian Nielsen -
Michael Widenius -
Oleksandr Byelkin