Percona Server with XtraDB

compiler warnings for 5.1.47-rel11.2

Reported by Mark Callaghan on 2010-07-27
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Server
Undecided
Oleg Tsarev

Bug Description

I built 5.1.470-rel11.2 from source. The build server is centos 5.2 using gcc 4.1.2

Configured it with:
./configure --enable-thread-safe-client --with-plugins=partition,csv,blackhole,myisam,heap,innodb_plugin --without-plugin-innobase --with-fast-mutexes --with-extra-charsets=all --with-debug C_EXTRA_FLAGS="-fno-omit-frame-pointer -fno-strict-aliasing -DHAVE_purify -Wall"

There are many compiler warnings in storage/innodb_plugin. I think all of this are new from Percona. I don't think that base InnoDB has these. Some of these are minor. Others are not.

btr/btr0btr.c:2928: warning: null argument where non-null required (argument 1)
btr/btr0cur.c:1834: warning: null argument where non-null required (argument 2)
btr/btr0cur.c:1853: warning: null argument where non-null required (argument 1)
btr/btr0cur.c:1960: warning: null argument where non-null required (argument 1)
btr/btr0cur.c:3415: warning: unused variable ‘j’
buf/buf0buf.c:3526: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 3 has type ‘unsigned int’
buf/buf0buf.c:81: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 4 has type ‘int’
buf/buf0buf.c:82: warning: comparison is always false due to limited range of data type
buf/buf0buf.c:83: warning: format ‘%lu’ expects type ‘long unsigned int’, but argument 4 has type ‘int’
fil/fil0fil.c:3125: warning: pointer targets in passing argument 2 of ‘dict_table_get_index_on_name’ differ in signedness
fil/fil0fil.c:3149: warning: unknown conversion type character ‘:’ in format
fil/fil0fil.c:3239: warning: implicit declaration of function ‘row_get_trx_id_offset’
fil/fil0fil.c:3326: warning: implicit declaration of function ‘que_eval_sql’
fsp/fsp0fsp.c:661:1: warning: multi-line comment
handler/ha_innodb.cc:2047: warning: comparison between signed and unsigned integer expressions
handler/i_s.cc:3040: warning: unused variable ‘cs’
ibuf/ibuf0ibuf.c:792: warning: null argument where non-null required (argument 1)
ibuf/ibuf0ibuf.c:967: warning: null argument where non-null required (argument 1)
./include/mtr0log.ic:206: warning: comparison between signed and unsigned integer expressions
log/log0recv.c:3328: warning: unused variable ‘file’
os/os0file.c:4188: warning: pointer targets in assignment differ in signedness
../../sql/handler.h:1125: warning: ‘ulonglong handler::rows_read’
../../sql/handler.h:1156: warning: ‘handler::auto_inc_intervals_count’ will be initialized after
../../sql/handler.h:1158: warning: when initialized here
srv/srv0srv.c:1121: warning: value computed is not used
srv/srv0srv.c:1151: warning: value computed is not used
srv/srv0srv.c:1358: warning: value computed is not used
srv/srv0srv.c:1851: warning: unused variable ‘io_counter_subtotal’
sync/sync0sync.c:437: warning: unused variable ‘ptr’
trx/trx0sys.c:1146: warning: unused variable ‘sys_header’
trx/trx0sys.c:1147: warning: unused variable ‘slot_no’
trx/trx0sys.c:1150: warning: unused variable ‘page_no’
trx/trx0sys.c:1151: warning: unused variable ‘i’

And these are warnings from the sql directory that I think are new. I will guess that some originated in the Google patch.

sql_show.cc:2416: warning: ‘int aggregate_user_stats(HASH*, HASH*)’ defined but not used
sql_show.cc:2428: warning: comparison between signed and unsigned integer expressions
sql_show.cc:2479: warning: format ‘%d’ expects type ‘int’, but argument 2 has type ‘ulong’
sql_show.cc:2479: warning: format ‘%d’ expects type ‘int’, but argument 3 has type ‘ulong’
sql_show.cc:2499: warning: comparison between signed and unsigned integer expressions
sql_show.cc:2535: warning: comparison between signed and unsigned integer expressions
sql_show.cc:2680: warning: comparison between signed and unsigned integer expressions
sql_show.cc:2723: warning: comparison between signed and unsigned integer expressions
sql_table.cc:1881: warning: ‘alias’ may be used uninitialized in this function
sql_table.cc:1882: warning: ‘path_length’ may be used uninitialized in this function

Oleg Tsarev (tsarev) on 2010-07-29
Changed in percona-server:
assignee: nobody → Oleg Tsarev (tsarev)
status: New → In Progress
Changed in percona-server:
milestone: none → 5.1-12.0
Oleg Tsarev (tsarev) wrote :

Confirmed

I checked a little.
Not all of the warnings are new at Percona.

At least,
btr/btr0btr.c:2928: warning: null argument where non-null required (argument 1)
btr/btr0cur.c:1834: warning: null argument where non-null required (argument 2)
btr/btr0cur.c:1853: warning: null argument where non-null required (argument 1)
btr/btr0cur.c:1960: warning: null argument where non-null required (argument 1)

These are original warning.

We will fix only about our patches of we can.

Oleg Tsarev (tsarev) wrote :

This is work for Yasufumi:
1:
btr/btr0btr.c: In function ‘btr_discard_only_page_on_level’:
btr/btr0btr.c:2911: warning: null argument where non-null required (argument 1)
btr/btr0cur.c: In function ‘btr_cur_update_alloc_zip’:
btr/btr0cur.c:1834: warning: null argument where non-null required (argument 2)
btr/btr0cur.c:1853: warning: null argument where non-null required (argument 1)
btr/btr0cur.c: In function ‘btr_cur_update_in_place’:
btr/btr0cur.c:1960: warning: null argument where non-null required (argument 1)
2:
buf/buf0buf.c: In function ‘_increment_page_get_statistics’:
buf/buf0buf.c:86: warning: comparison is always false due to limited range of data type
buf/buf0buf.c: In function ‘buf_chunk_init’:
buf/buf0buf.c:840: warning: ‘zip_hash_tmp’ may be used uninitialized in this function
3:
fil/fil0fil.c: In function ‘fil_open_single_table_tablespace’:
fil/fil0fil.c:3128: warning: pointer targets in passing argument 2 of ‘dict_table_get_index_on_name’ differ in signedness
4:
ibuf/ibuf0ibuf.c: In function ‘ibuf_set_free_bits_low’:
ibuf/ibuf0ibuf.c:792: warning: null argument where non-null required (argument 1)
ibuf/ibuf0ibuf.c: In function ‘ibuf_update_free_bits_zip’:
ibuf/ibuf0ibuf.c:967: warning: null argument where non-null required (argument 1)

Oleg Tsarev (tsarev) wrote :
Changed in percona-server:
assignee: Oleg Tsarev (tsarev) → Yasufumi Kinoshita (yasufumi-kinoshita)
Mark Callaghan (mdcallag) wrote :

How do you reproduce the warnings in 5.1.47 storage/innodb_plugin? I cannot after using unmodified 5.1.47 and

./configure --enable-thread-safe-client --with-plugins=partition,csv,blackhole,myisam,heap,innodb_plugin --without-plugin-innobase --with-fast-mutexes --with-extra-charsets=all --with-debug C_EXTRA_FLAGS="-fno-omit-frame-pointer -fno-strict-aliasing -DHAVE_purify -DNO_ALARM -DSIGNAL_WITH_VIO_CLOSE -Wall"

Mark Callaghan (mdcallag) wrote :

One of the warnings is from btr0btr line 2928. The call is to:

/************************************************************//**
Determine whether the page is a B-tree leaf.
@return TRUE if the page is a B-tree leaf */
UNIV_INLINE
ibool
page_is_leaf(
/*=========*/
        const page_t* page) /*!< in: page */
        __attribute__((nonnull, pure));

This uses __attribute__ nonnull.

The warning is for:

                if (page_is_leaf(buf_block_get_frame(block))) {

The warning went away when I split this into two lines:
f= buf_block_get_frame(block)
if (page_is_leaf(f)))

This is odd.

Mark Callaghan (mdcallag) wrote :

The warnings about null are from your patch

Your patch changed buf_block_get_frame from this:
#ifdef UNIV_DEBUG
/*********************************************************************//**
Gets a pointer to the memory frame of a block.
@return pointer to the frame */
UNIV_INLINE
buf_frame_t*
buf_block_get_frame(
/*================*/
        const buf_block_t* block) /*!< in: pointer to the control block */
        __attribute__((pure));
#else /* UNIV_DEBUG */
# define buf_block_get_frame(block) (block)->frame
#endif /* UNIV_DEBUG */

to this, check the else branch

#ifdef UNIV_DEBUG
 848 /*********************************************************************//**
 849 Gets a pointer to the memory frame of a block.
 850 @return pointer to the frame */
 851 UNIV_INLINE
 852 buf_frame_t*
 853 buf_block_get_frame(
 854 /*================*/
 855 const buf_block_t* block) /*!< in: pointer to the control block */
 856 __attribute__((pure));
 857 #else /* UNIV_DEBUG */
 858 # define buf_block_get_frame(block) (block ? (block)->frame : 0)
 859 #endif /* UNIV_DEBUG */

Thank you.
I was wrong. I will fix the rests also.

Oleg,

I have fixed the most of warnings.
And pushed to the branch.
Pleas check the rests.

The rests are...

mysqld.cc: In function 'void init_cachedev()':
mysqld.cc:4402: warning: deprecated conversion from string constant to 'char*'
mysqld.cc:4408: warning: deprecated conversion from string constant to 'char*'
mysqld.cc:4415: warning: deprecated conversion from string constant to 'char*'
mysqld.cc:4430: warning: deprecated conversion from string constant to 'char*'
mysqld.cc:4437: warning: deprecated conversion from string constant to 'char*'
mysqld.cc:4446: warning: deprecated conversion from string constant to 'char*'

---> sql_no_fcache.patch

mysqld.cc: In function 'void* handle_connections_sockets(void*)':
mysqld.cc:5221: warning: 'sock' may be used uninitialized in this function
mysqld.cc:5225: warning: 'flags' may be used uninitialized in this function

---> bugfix48929.patch ?
(But I think it "will" be original warning... and not need to be fixed.
 Because the patch is backport from the commit to the next version of mysql)

query_response_time.cc: In member function 'void query_response_time::string_collector::setup(query_response_time::utility&)':
query_response_time.cc:159: warning: ' ' flag used with '%u' gnu_printf format
query_response_time.cc:159: warning: ' ' flag used with '%u' gnu_printf format

---> response-time-distribution.patch

Changed in percona-server:
assignee: Yasufumi Kinoshita (yasufumi-kinoshita) → Oleg Tsarev (tsarev)
Mark Callaghan (mdcallag) wrote :

> mysqld.cc:4446: warning: deprecated conversion from string constant to 'char*'
>
> ---> sql_no_fcache.patch

This is from the facebook patch. I need to fix that. It is easy to spot new warnings for InnoDB, but unmodified MySQL has many to begin with.

Mark,

I compared the differential about .....

grep -i warning build.log | sed -e 's/:[0-9]*://'
outputs of build logs to find the additional warnings.

Oleg,

In the end, please fix only response-time-distribution.patch about following warnings if you can.

query_response_time.cc: In member function 'void query_response_time::string_collector::setup(query_response_time::utility&)':
query_response_time.cc:159: warning: ' ' flag used with '%u' gnu_printf format
query_response_time.cc:159: warning: ' ' flag used with '%u' gnu_printf format

Oleg Tsarev (tsarev) wrote :

Yasudumi, i fixed warning in response-time-distribution.
Your turn.
Do you complete this branch?

Changed in percona-server:
assignee: Oleg Tsarev (tsarev) → Yasufumi Kinoshita (yasufumi-kinoshita)

Thank you, Oleg.

Do you find any other warnings?
I think it is OK, at least about our patches.

And the "line number alignment" should be later, because of ease to merge.

Please propose to merge if we hurry.
I am OK for change of the branch to be merged.

Oleg Tsarev (tsarev) wrote :

Yasufumi,

We have some warning from sql_no_fcache.patch, but Mark want to fix.
Three ways:
1) I can fix sql_no_fcache.patch
2) We can wait while Mark fix.
3) We can merge without this fix of sql_no_fcache.patch.

How do you think, what better?

I think 3) is OK for now.
The warnings are acceptable for me.

Oleg Tsarev (tsarev) wrote :

Ok. i propose for merge ths fixes.

Changed in percona-server:
assignee: Yasufumi Kinoshita (yasufumi-kinoshita) → Oleg Tsarev (tsarev)
Oleg Tsarev (tsarev) on 2010-08-16
Changed in percona-server:
status: In Progress → Fix Committed
Changed in percona-server:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers