Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up c-style formatting #3599

Merged
merged 6 commits into from
Feb 27, 2019
Merged

Conversation

rwy7
Copy link
Contributor

@rwy7 rwy7 commented Feb 21, 2019

Introduce a new header, omrformatconsts.h, which defines a set of portable constants for building format strings for special integral types. This header provides platform-independent format specifiers for the following data types:

  • int8_t / uint8_t
  • int16_t / uint16_t
  • int32_t / uint32_t
  • int64 / uint64_t
  • size_t
  • intptr_t / uintptr_t
  • ptrdiff_t

The following formats are provided in this PR:

  • x: unsigned hexadecimal format
  • u: unsigned decimal format
  • d: signed decimal format

(notably missing: i)

Normally, there are either special macros defined by the C library, or
special format characters in the case of size_t / ptrdiff_t.
Unfortunately, because of poor standards compliance across our
compilers, these aren't universally available.

The names of the constants follow the convention of those normally
provided by stdint.h.

As well, I've gone through and fixed a number of locations with format strings that generate errors.

@rwy7 rwy7 changed the title Fix format strings Clean up c-style formatting Feb 21, 2019
@Leonardo2718
Copy link
Contributor

@genie-omr build all

@Leonardo2718 Leonardo2718 self-assigned this Feb 22, 2019
@@ -21,6 +21,7 @@

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copyright date needs to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I might wait until CI passes, then push an amended commit that updates the copyrights, if that's OK?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, although the zos machines are down so it might be a while before that job passes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

rwy7 added 6 commits February 25, 2019 16:43
This PR adds a new header, omrformatconsts.h. This header  provides
platform-independent format specifiers for the following data types:

- int8_t / uint8_t
- int16_t / uint16_t
- int32_t / uint32_t
- int64 / uint64_t
- size_t
- intptr_t / uintptr_t
- ptrdiff_t

The following formats are provided in this PR:
- x: unsigned hexadecimal format
- u: unsigned decimal format
- d: signed decimal format

Normally, there are either special macros defined by the C library, or
special format characters in the case of size_t / ptrdiff_t.
Unfortunately, because of poor standards compliance across our
compilers, these aren't universally available.

This PR defines portable string constants that can be used to contruct
correct format strings.

The names of the constants follow the convention of those normally
provided by stdint.h.

Signed-off-by: Robert Young <rwy0717@gmail.com>
Signed-off-by: Robert Young <rwy0717@gmail.com>
Signed-off-by: Robert Young <rwy0717@gmail.com>
Signed-off-by: Robert Young <rwy0717@gmail.com>
The PID is, wierdly enough, a 32bit signed integer on most platforms.
Extend the PID type to a signed long, just in case there's a platform
that actually uses 64 bit pids.

Signed-off-by: Robert Young <rwy0717@gmail.com>
Signed-off-by: Robert Young <rwy0717@gmail.com>
@rwy7 rwy7 force-pushed the fix-format-strings branch from 28c1d1a to f38f459 Compare February 25, 2019 21:54
@rwy7
Copy link
Contributor Author

rwy7 commented Feb 25, 2019

  • Replaced __xlc__ test with __xlC__.
  • Fixed copyright in MaxMinTest.cpp
  • Rebased changes

Copy link
Contributor

@Leonardo2718 Leonardo2718 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Leonardo2718
Copy link
Contributor

@genie-omr build all

@Leonardo2718
Copy link
Contributor

@genie-omr build zos

@Leonardo2718
Copy link
Contributor

Alright, the zos build is failing because of infrastructural problems. These changes are not z specific and all the other builds (including zlinux) passed. Merging.

@Leonardo2718 Leonardo2718 merged commit 6e7d6bf into eclipse-omr:master Feb 27, 2019
@rwy7 rwy7 deleted the fix-format-strings branch February 27, 2019 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants