compiler warnings for 5.1.47-rel11.2

Bug #610525 reported by Mark Callaghan
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Percona Server moved to https://jira.percona.com/projects/PS
Fix Released
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)
Changed in percona-server:
assignee: nobody → Oleg Tsarev (tsarev)
status: New → In Progress
Changed in percona-server:
milestone: none → 5.1-12.0
Revision history for this message
Oleg Tsarev (tsarev) wrote :

Confirmed

Revision history for this message
Yasufumi Kinoshita (yasufumi-kinoshita) wrote :

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.

Revision history for this message
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)

Revision history for this message
Oleg Tsarev (tsarev) wrote :
Changed in percona-server:
assignee: Oleg Tsarev (tsarev) → Yasufumi Kinoshita (yasufumi-kinoshita)
Revision history for this message
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"

Revision history for this message
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.

Revision history for this message
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 */

Revision history for this message
Yasufumi Kinoshita (yasufumi-kinoshita) wrote :

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

Revision history for this message
Yasufumi Kinoshita (yasufumi-kinoshita) wrote :

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)
Revision history for this message
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.

Revision history for this message
Yasufumi Kinoshita (yasufumi-kinoshita) wrote :

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

Revision history for this message
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)
Revision history for this message
Yasufumi Kinoshita (yasufumi-kinoshita) wrote :

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.

Revision history for this message
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?

Revision history for this message
Yasufumi Kinoshita (yasufumi-kinoshita) wrote :

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

Revision history for this message
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)
Changed in percona-server:
status: In Progress → Fix Committed
Changed in percona-server:
status: Fix Committed → Fix Released
Revision history for this message
Shahriyar Rzayev (rzayev-sehriyar) wrote :

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PS-2545

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.