Skip to content

Fix compiler warnings, part 2#1256

Closed
nilason wants to merge 2 commits into
OSGeo:masterfrom
nilason:fix-compiler-warnings-2
Closed

Fix compiler warnings, part 2#1256
nilason wants to merge 2 commits into
OSGeo:masterfrom
nilason:fix-compiler-warnings-2

Conversation

@nilason

@nilason nilason commented Jan 17, 2021

Copy link
Copy Markdown
Contributor

Fixes -Wformat compiler warnings.

One important culprit to these warnings was caused by bad detection of off_t print format with PRI_OFF_T.
Suggested solution works for me. I'm curious if it bears in general.

Second part addressing #1247.

Update:

The PRI_OFF_T problem seems to be Mac-only, updated accordingly.

Modules / code parts directly affected:

  • db/drivers/ogr
  • lib/vector/Vlib
  • raster/r.compress
  • raster/r.info
  • raster/t.terraflow
  • raster/r.viewshed
  • raster3d/r3.out.vtk
  • raster3d/r3.stats
  • tools/timer
  • vector/v.in.ascii
  • vector/v.lrs/v.lrs.create

@nilason nilason mentioned this pull request Jan 17, 2021
27 tasks
@nilason

nilason commented Jan 17, 2021

Copy link
Copy Markdown
Contributor Author

The CIs failing because of intmax_t, will see to that ASAP.

Comment thread configure.in Outdated
Comment thread include/gis.h Outdated
Comment thread lib/vector/Vlib/build_nat.c Outdated
Comment thread raster/r.info/main.c Outdated
Comment thread raster/r.info/main.c Outdated
Comment thread raster/r.info/main.c Outdated
@metzm

metzm commented Jan 17, 2021

Copy link
Copy Markdown
Contributor

The CIs failing because of intmax_t, will see to that ASAP.

Use grass_int64 instead of intmax_t.

@nilason nilason force-pushed the fix-compiler-warnings-2 branch from 408a638 to 9b4e30d Compare January 18, 2021 12:20
@nilason nilason marked this pull request as draft January 18, 2021 13:13
@nilason nilason force-pushed the fix-compiler-warnings-2 branch from 9b4e30d to 44184ca Compare January 18, 2021 17:52
@nilason

nilason commented Jan 22, 2021

Copy link
Copy Markdown
Contributor Author

The CIs failing because of intmax_t, will see to that ASAP.

Use grass_int64 instead of intmax_t.

I restored grass_int64, but as printf() doesn't recognise that type, I use PRId64 macro for the format. Most platforms (all on CI) seem to support this.

@metzm

metzm commented Jan 27, 2021

Copy link
Copy Markdown
Contributor

I restored grass_int64, but as printf() doesn't recognise that type, I use PRId64 macro for the format. Most platforms (all on CI) seem to support this.

grass_int64 is not a fixed type, but a platform-specific typedef ... grass_int64 for some available 64 bit integer type. If PRId64 works across platforms, great! If need be, we can define matching macros in gis.h.

The aim to eliminate compiler warnings is great, but bear in mind that some compiler warnings are nonsense, requiring case-by-case judgement.

Comment thread configure Outdated
Comment thread lib/vector/Vlib/build_nat.c Outdated
Comment thread raster3d/r3.out.vtk/errorHandling.c Outdated
Comment thread raster3d/r3.stats/support.c Outdated
Comment thread tools/timer/main.c
@nilason

nilason commented Jan 28, 2021

Copy link
Copy Markdown
Contributor Author

I restored grass_int64, but as printf() doesn't recognise that type, I use PRId64 macro for the format. Most platforms (all on CI) seem to support this.

grass_int64 is not a fixed type, but a platform-specific typedef ... grass_int64 for some available 64 bit integer type. If PRId64 works across platforms, great! If need be, we can define matching macros in gis.h.

The aim to eliminate compiler warnings is great, but bear in mind that some compiler warnings are nonsense, requiring case-by-case judgement.

PRId64 is declared in <inttypes.h> and therefore dependent on C99 support. Independent of outcome regarding statement on standard Cxx usage it may be implemented in "gis.h" as you suggested. Maybe like G_PRId64? That way it (inttypes.h) need not be included specifically when needed. If you agree, I will put up a suggestion for this.

@nilason

nilason commented Jan 28, 2021

Copy link
Copy Markdown
Contributor Author

The aim to eliminate compiler warnings is great, but bear in mind that some compiler warnings are nonsense, requiring case-by-case judgement.

I'm aware many compilation warnings are just annoyances, but if there are acceptable ways of getting rid of them, all the better.

@metzm

metzm commented Jan 29, 2021

Copy link
Copy Markdown
Contributor

I'm aware many compilation warnings are just annoyances, but if there are acceptable ways of getting rid of them, all the better.

I agree, and try to help to sieve through these compiler warnings. For some of these warnings, I just don't know how to get rid off them without breaking compatibility.

@nilason

nilason commented Feb 8, 2021

Copy link
Copy Markdown
Contributor Author

Updated according to your initial review.

The question on PRId64 and <inttypes.h> remains to be cleared. No need to force this, just wanted to fix the other issues.

@metzm

metzm commented Feb 8, 2021

Copy link
Copy Markdown
Contributor

The question on PRId64 and <inttypes.h> remains to be cleared. No need to force this, just wanted to fix the other issues.

Great, please merge! Let's leave the question on PRId64 and `<inttypes.h> for another PR until there is agreement on the dev ml.

_FILE_OFFSET_BITS is not set for Mac compiling with clang, but off_t is
defined as a int64 (long long) type requiring a "lld" printf format.
nilason added a commit to nilason/grass that referenced this pull request Feb 9, 2021
Addresses -Wformat compiler warnings.
@nilason nilason force-pushed the fix-compiler-warnings-2 branch from 51e259d to 5c16319 Compare February 9, 2021 08:23
@nilason

nilason commented Feb 9, 2021

Copy link
Copy Markdown
Contributor Author

Let's leave the question on PRId64 and `<inttypes.h> for another PR until there is agreement on the dev ml.

Lifted out and moved this part to #1316.

Addresses -Wformat compiler warnings.
@nilason nilason force-pushed the fix-compiler-warnings-2 branch from 5c16319 to 046db74 Compare February 9, 2021 09:24
@nilason nilason marked this pull request as ready for review February 9, 2021 09:41
@nilason

nilason commented Feb 9, 2021

Copy link
Copy Markdown
Contributor Author

Ready, will merge on approval.

@metzm metzm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good now.

nilason added a commit that referenced this pull request Feb 13, 2021
_FILE_OFFSET_BITS is not set for Mac compiling with clang, but off_t is
defined as a int64 (long long) type requiring a "lld" printf format.
nilason added a commit that referenced this pull request Feb 13, 2021
Addresses -Wformat compiler warnings.
@nilason

nilason commented Feb 13, 2021

Copy link
Copy Markdown
Contributor Author

Thanks a lot. Cherry picked and pushed manually.

@nilason nilason closed this Feb 13, 2021
@nilason nilason deleted the fix-compiler-warnings-2 branch February 14, 2021 13:03
marisn pushed a commit to marisn/grass that referenced this pull request Mar 22, 2021
_FILE_OFFSET_BITS is not set for Mac compiling with clang, but off_t is
defined as a int64 (long long) type requiring a "lld" printf format.
marisn pushed a commit to marisn/grass that referenced this pull request Mar 22, 2021
Addresses -Wformat compiler warnings.
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
_FILE_OFFSET_BITS is not set for Mac compiling with clang, but off_t is
defined as a int64 (long long) type requiring a "lld" printf format.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022
Addresses -Wformat compiler warnings.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
_FILE_OFFSET_BITS is not set for Mac compiling with clang, but off_t is
defined as a int64 (long long) type requiring a "lld" printf format.
ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023
Addresses -Wformat compiler warnings.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants