[MDEV-12488] Remove type mismatch in InnoDB printf-like calls - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Major
Resolution: Fixed
Affects Version/s: 10.2(EOL)
Fix Version/s: 10.1.23, 10.0.31, 10.2.6
Component/s: Compiling, Storage Engine - InnoDB
Labels:
None
Environment:
Osx clang

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:

[1m/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:4: [0m[0;1;35mwarning: [0m[1mformat specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat][0m

                        m_low_limit_id, m_up_limit_id);

[0;1;32m                        ^~~~~~~~~~~~~~

[0m[1m/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:20: [0m[0;1;35mwarning: [0m[1mformat specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat][0m

                        m_low_limit_id, m_up_limit_id);

[0;1;32m                                        ^~~~~~~~~~~~~

Attachments

Issue Links

relates to

MDEV-12534 Use atomic operations whenever available

Closed

Activity

Ascending order - Click to sort in descending order

Daniel Black created issue - 2017-04-11 22:22

Elena Stepanova made changes - 2017-04-12 01:57

Field	Original Value	New Value
Component/s		Compiling [ 11001 ]
Fix Version/s		10.2 [ 14601 ]
Assignee		Marko Mäkelä [ marko ]

Daniel Black made changes - 2017-04-12 03:13

Description

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

{noformat}[0mIn 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: [1m/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:4: [0m[0;1;35mwarning: [0m[1mformat specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat][0m m_low_limit_id, m_up_limit_id); [0;1;32m ^~~~~~~~~~~~~~ [0m[1m/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:20: [0m[0;1;35mwarning: [0m[1mformat specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat][0m m_low_limit_id, m_up_limit_id); [0;1;32m ^~~~~~~~~~~~~ [0m3 warnings generated.
,{noformat}

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

{noformat}
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:

[1m/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:4: [0m[0;1;35mwarning: [0m[1mformat specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat][0m

m_low_limit_id, m_up_limit_id);

[0;1;32m ^~~~~~~~~~~~~~

[0m[1m/Users/travis/build/grooverdan/mariadb-server/storage/innobase/include/read0types.h:220:20: [0m[0;1;35mwarning: [0m[1mformat specifies type 'unsigned long' but the argument has type 'trx_id_t' (aka 'unsigned long long') [-Wformat][0m

m_low_limit_id, m_up_limit_id);

[0;1;32m ^~~~~~~~~~~~~
,{noformat}

Marko Mäkelä made changes - 2017-04-19 11:27

Status

Open [ 1 ]

In Progress [ 3 ]

Marko Mäkelä added a comment - 2017-04-19 12:28

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

Marko Mäkelä added a comment - 2017-04-19 12:28 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()

Marko Mäkelä made changes - 2017-04-19 12:52

Summary

compile warning - wrong size of(trx_id_t)

Remove type mismatch in InnoDB printf-like calls

Marko Mäkelä added a comment - 2017-04-19 12:53 - edited

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.

Marko Mäkelä added a comment - 2017-04-19 12:53 - edited 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.

Marko Mäkelä made changes - 2017-04-19 12:53

Assignee	Marko Mäkelä [ marko ]	Jan Lindström [ jplindst ]
Status	In Progress [ 3 ]	In Review [ 10002 ]

Marko Mäkelä made changes - 2017-04-20 03:41

Link

This issue relates to ~~MDEV-12534~~ [ ~~MDEV-12534~~ ]

Daniel Black added a comment - 2017-04-20 07:35

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

Daniel Black added a comment - 2017-04-20 07:35 Ack. Thank you. Sorry it was so invasive a fix.

Rasmus Johansson (Inactive) made changes - 2017-04-20 08:08

Sprint

10.1.23 [ 154 ]

Rasmus Johansson (Inactive) made changes - 2017-04-20 08:08

Rank

Ranked higher

Marko Mäkelä added a comment - 2017-04-20 13:53

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.

Marko Mäkelä added a comment - 2017-04-20 13:53 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.

Marko Mäkelä added a comment - 2017-04-20 17:08

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

Marko Mäkelä added a comment - 2017-04-20 17:08 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

Marko Mäkelä added a comment - 2017-04-21 03:00 - edited

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

Marko Mäkelä added a comment - 2017-04-21 03:00 - edited 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".

Daniel Black added a comment - 2017-04-21 04:43 - edited

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

10.0: https://travis-ci.org/grooverdan/mariadb-server/jobs/224218995
10.1: https://travis-ci.org/grooverdan/mariadb-server/jobs/224218529 (line 6139)

(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

Daniel Black added a comment - 2017-04-21 04:43 - edited 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 10.0: https://travis-ci.org/grooverdan/mariadb-server/jobs/224218995 10.1: https://travis-ci.org/grooverdan/mariadb-server/jobs/224218529 (line 6139) (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

Jan Lindström (Inactive) added a comment - 2017-04-21 07:48 - edited

Hi, 10.0/10.1 is ok to push.

Jan Lindström (Inactive) added a comment - 2017-04-21 07:48 - edited Hi, 10.0/10.1 is ok to push.

Marko Mäkelä added a comment - 2017-04-21 10:29

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

Marko Mäkelä added a comment - 2017-04-21 10:29 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

Jan Lindström (Inactive) added a comment - 2017-04-21 11:41

ok to push 10.2.

Jan Lindström (Inactive) added a comment - 2017-04-21 11:41 ok to push 10.2.

Jan Lindström (Inactive) made changes - 2017-04-21 11:41

Assignee	Jan Lindström [ jplindst ]	Marko Mäkelä [ marko ]
Status	In Review [ 10002 ]	Stalled [ 10000 ]

Marko Mäkelä made changes - 2017-04-21 15:32

issue.field.resolutiondate

2017-04-21 15:32:15.0

2017-04-21 15:32:15.232

Marko Mäkelä made changes - 2017-04-21 15:32

Fix Version/s		10.0.31 [ 22501 ]
Fix Version/s		10.1.23 [ 22503 ]
Fix Version/s		10.2.6 [ 22527 ]
Fix Version/s	10.2 [ 14601 ]
Resolution		Fixed [ 1 ]
Status	Stalled [ 10000 ]	Closed [ 6 ]

Daniel Black added a comment - 2017-04-24 01:12

Thanks again.

Daniel Black added a comment - 2017-04-24 01:12 Thanks again.

Sergei Golubchik made changes - 2021-12-06 21:44

Workflow

MariaDB v3 [ 80304 ]

MariaDB v4 [ 151947 ]

People

Assignee:: Marko Mäkelä

Reporter:: Daniel Black

Votes:: 0 Vote for this issue

Watchers:: 3 Start watching this issue

Dates

Created:: 2017-04-11 22:22

Updated:: 2017-04-24 05:31

Resolved:: 2017-04-21 15:32

Git Integration

Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.

MariaDB Server

Details

Description

Attachments

Issue Links

Activity

People

Dates

Git Integration