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.

Sign up for GitHub

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

Jump to bottom

config: set -Wno-format-truncation globally #15908

Closed
wants to merge 2 commits into from
Closed

config: set -Wno-format-truncation globally #15908

wants to merge 2 commits into from

Conversation

robn
Copy link
Member

@robn robn commented Feb 19, 2024 •
edited
Loading

Motivation and Context

-Wformat-truncation looks for places where the return code of snprintf() is unchecked and the provided buffer might be too short. This is based on a heuristic that can change between compiler versions.

It has been seen to get this wrong in ddt_object_name(), leading to DDT_NAMELEN being increased somewhat arbitrarily.

There's no good reason to have this warning enabled, so here we disable it everywhere. Truncation may be undesirable, but snprintf() is guaranteed to emit a trailing null, so at worst we get a short string, not a buffer overrun.

Closes #15905.

Description

Add NO_FORMAT_TRUNCATION to AM_CFLAGS globally.

Reduce DDT_NAMELEN to something sensible, since we're no longer blowing up on it.

How Has This Been Tested?

Compiles locally, quick dedup check looks right.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@robn
config: use -Wno-format-truncation globally
28054c8 
-Wformat-truncation looks for places where the return code of snprintf()
is unchecked and the provided buffer might be too short. This is based
on a heuristic that can change between compiler versions.

It has been seen to get this wrong in ddt_object_name(), leading to
DDT_NAMELEN being increased somewhat arbitrarily.

There's no good reason to have this warning enabled, so here we disable
it everywhere. Truncation may be undesirable, but snprintf() is
guaranteed to emit a trailing null, so at worst we get a short string,
not a buffer overrun.

Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
@robn
ddt: reduce DDT_NAMELEN
220c772 
This is the buffer size passed to ddt_object_name(), to expand the
DMU_POOL_DDT format. That format inserts the table checksum, class and
type names, which as I write this are max 6, 9 and 3, respectively.

Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
@robn robn force-pushed the config-force-no-format-truncation branch from 0c402ff to 220c772 Compare February 19, 2024 10:43
@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Feb 26, 2024
behlendorf pushed a commit that referenced this pull request Feb 26, 2024
@robn @behlendorf
ddt: reduce DDT_NAMELEN
8f2f6cd 
This is the buffer size passed to ddt_object_name(), to expand the
DMU_POOL_DDT format. That format inserts the table checksum, class and
type names, which as I write this are max 6, 9 and 3, respectively.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes #15908
@behlendorf behlendorf mentioned this pull request Feb 26, 2024
Closed
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@robn @lundman
config: use -Wno-format-truncation globally
85a08a5 
-Wformat-truncation looks for places where the return code of snprintf()
is unchecked and the provided buffer might be too short. This is based
on a heuristic that can change between compiler versions.

It has been seen to get this wrong in ddt_object_name(), leading to
DDT_NAMELEN being increased somewhat arbitrarily.

There's no good reason to have this warning enabled, so here we disable
it everywhere. Truncation may be undesirable, but snprintf() is
guaranteed to emit a trailing null, so at worst we get a short string,
not a buffer overrun.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#15908
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@robn @lundman
ddt: reduce DDT_NAMELEN
6c386c8 
This is the buffer size passed to ddt_object_name(), to expand the
DMU_POOL_DDT format. That format inserts the table checksum, class and
type names, which as I write this are max 6, 9 and 3, respectively.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#15908
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@robn @lundman
config: use -Wno-format-truncation globally
28be437 
-Wformat-truncation looks for places where the return code of snprintf()
is unchecked and the provided buffer might be too short. This is based
on a heuristic that can change between compiler versions.

It has been seen to get this wrong in ddt_object_name(), leading to
DDT_NAMELEN being increased somewhat arbitrarily.

There's no good reason to have this warning enabled, so here we disable
it everywhere. Truncation may be undesirable, but snprintf() is
guaranteed to emit a trailing null, so at worst we get a short string,
not a buffer overrun.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#15908
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@robn @lundman
ddt: reduce DDT_NAMELEN
91781f8 
This is the buffer size passed to ddt_object_name(), to expand the
DMU_POOL_DDT format. That format inserts the table checksum, class and
type names, which as I write this are max 6, 9 and 3, respectively.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#15908
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@robn @lundman
config: use -Wno-format-truncation globally
04d7872 
-Wformat-truncation looks for places where the return code of snprintf()
is unchecked and the provided buffer might be too short. This is based
on a heuristic that can change between compiler versions.

It has been seen to get this wrong in ddt_object_name(), leading to
DDT_NAMELEN being increased somewhat arbitrarily.

There's no good reason to have this warning enabled, so here we disable
it everywhere. Truncation may be undesirable, but snprintf() is
guaranteed to emit a trailing null, so at worst we get a short string,
not a buffer overrun.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#15908
lundman pushed a commit to openzfsonwindows/openzfs that referenced this pull request Mar 13, 2024
@robn @lundman
ddt: reduce DDT_NAMELEN
cf7a51b 
This is the buffer size passed to ddt_object_name(), to expand the
DMU_POOL_DDT format. That format inserts the table checksum, class and
type names, which as I write this are max 6, 9 and 3, respectively.

Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Rob Norris <robn@despairlabs.com>
Sponsored-by: https://despairlabs.com/sponsor/
Closes openzfs#15908
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

Assignees
No one assigned
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Warning from ddt_load breaking -Werror
2 participants
@robn @behlendorf