[MDEV-12488] Remove type mismatch in InnoDB printf-like calls Created: 2017-04-11  Updated: 2017-04-24  Resolved: 2017-04-21

Status: Closed
Project: MariaDB Server
Component/s: Compiling, Storage Engine - InnoDB
Affects Version/s: 10.2
Fix Version/s: 10.1.23, 10.0.31, 10.2.6

Type: Bug Priority: Major
Reporter: Daniel Black Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: None
Environment:

Osx clang


Issue Links:
Relates
relates to MDEV-12534 Use atomic operations whenever available Closed
Sprint: 10.1.23

 Description   

https://s3.amazonaws.com/archive.travis-ci.org/jobs/220849959/log.txt

In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/btr/btr0btr.cc:41:
 
In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/lock0lock.h:1112:
 
In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/lock0lock.ic:29:
 
In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/row0row.h:390:
 
In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/row0row.ic:28:
 
In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/trx0undo.h:34:
 
In file included from /Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/trx0sys.h:40:
 
/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:4: warning: format specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat]
 
                        m_low_limit_id, m_up_limit_id);
 
                        ^~~~~~~~~~~~~~
 
/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:20: warning: format specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat]
 
                        m_low_limit_id, m_up_limit_id);
 
                                        ^~~~~~~~~~~~~
,



 Comments   
Comment by Marko Mäkelä [ 2017-04-19 ]

Apparently, the problem is an inconsistency between <inttypes.h> and <stdint.h> in the Mac OS X headers, for uint64_t and PRIu64.
I think that we should use long long int as the 64-bit type on Mac OS X.

That said, I am testing a fix for 10.0 that does some further cleanup:

typedef size_t ulint;
#define ULINTPF "%zu"
// adjust all printf format strings to use ULINTPF, and remove many type casts
// make more use of ib_logf()

Comment by Marko Mäkelä [ 2017-04-19 ]

bb-10.0-marko, merged to bb-10.1-marko.
I think that we must consider what is feasible in GA versions. Maybe we should limit the bulk of these changes to 10.2.
In any case, for future reference, I will merge the patch via 10.1 to 10.2, but we do not have to push it to 10.0 or 10.1.

Comment by Daniel Black [ 2017-04-20 ]

Ack. Thank you. Sorry it was so invasive a fix.

Comment by Marko Mäkelä [ 2017-04-20 ]

danblack, no problem. We have to bite the bullet some time. For 10.2 it will be a smaller patch, because ib_logf() has been replaced with a C++ class.
This should also greatly reduce the number of warnings emitted on Windows.

Comment by Marko Mäkelä [ 2017-04-20 ]

A test merge to bb-10.2-marko was pushed.
I plan to push MDEV-12534 (which my commit is on top of) to 10.0, merge to 10.1, and then push this full MDEV-12488 patch only to 10.2.

Maybe in 10.0 it is feasible to do a minimal tweak of the 64-bit types for Mac OS X, and keep ulint and most of the other stuff intact.
There are a few bug fixes that can and should be done in 10.0 and 10.1:

  • correct an error message for IMPORT TABLESPACE (also remove the rdiff file for an import test)
  • correct a type mismatch in a debug message in row0merge.cc
  • in 10.1, make fil_space_acquire_silent() identical between InnoDB and XtraDB
Comment by Marko Mäkelä [ 2017-04-21 ]

Reduced version: bb-10.0-marko, also merged to bb-10.1-marko
Larger version (typedef size_t ulint): bb-10.2-marko
danblack, I now realize that maybe the warnings on Mac OS X were only issued for MariaDB 10.2. Can you confirm this? 10.0 and 10.1 seem to use PRIu64, while 10.2 seemingly worked around a Red Hat issue in the wrong way, by not defining the feature macro __STDC_FORMAT_MACROS but by replacing PRIu64 with "llu".

Comment by Daniel Black [ 2017-04-21 ]

I just picked up on the OSX errors after looking on Travis (haven't got hardware myself).

Seems almost ok for -Wformat for 10.0 with one error on 10.1

(bb-10.2-marko in progress https://travis-ci.org/MariaDB/server/builds/224207229).

10.1 warnings seem to be less and in different spots: http://buildbot.askmonty.org/buildbot/builders/mac-1012-bintar/builds/1278

Comment by Jan Lindström (Inactive) [ 2017-04-21 ]

Hi, 10.0/10.1 is ok to push.

Comment by Marko Mäkelä [ 2017-04-21 ]

Thanks, I pushed the 10.0 and merged to 10.1, with some problems from danblack’s build logs addressed.

Revised 10.2 patch: bb-10.2-marko

Comment by Jan Lindström (Inactive) [ 2017-04-21 ]

ok to push 10.2.

Comment by Daniel Black [ 2017-04-24 ]

Thanks again.

Generated at Thu Feb 08 07:58:07 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.