On 2016.12.23 at 14:25 -0700, Martin Sebor wrote:
> Bug 78703 points out that the decimal point character in floating
> directives can be longer than just one byte (in locales where the
> decimal point is a multibyte character).  The decimal point can
> result in anywhere between 1 and MB_LEN_MAX bytes.  This is unlikely
> but locales with two-byte decimal point are known to exist, and
> the gimple-ssa-sprintf pass must handle them correctly.
> 
> In a comment on the bug Jakub suggests that while printf return
> value optimization must correctly deal with the worst case (i.e.,
> MB_LEN_MAX of 6 for UTF-8), reflecting the worst case in the text
> of warnings could be confusing to users most of whom expect
> a single byte decimal point.
> 
> Finally, a limitation of the gimple-ssa-sprintf pass has been that
> it only understands constant width and precision and treats others
> as essentially unlimited even if they are constrained to a limited
> range of values.  This results in false positives and negatives
> that can be avoided.
> 
> The attached patch enhances the pass to overcome both of these
> limitations.  It does that by first replacing the exact byte counter
> with two other counters: 1) a likely counter that tracks the number
> of bytes a directive is likely to result in, and 2) an "unlikely"
> byte for lack of a better name, that tracks the unlikely maximum
> byte count in cases like multibyte decimal point, and second by
> adding range handling for width and precision specified by the
> asterisk (such as in sprintf("%*.*i", w, p, i)).
> 
> The patch resulted in more extensive changes than I initially
> intended but the result is a simplified implementation.  A good
> amount of the changes is factoring code out into more general
> functions that can be shared throughout the pass.
> 
> With these enhancements, although the support for ranges in the
> pass is complete, it's not as robust as it could be.  I think
> having the pass run later could improve things.
> 
> The pass does produce a fair number of warnings for calls to
> snprintf in the linux kernel.  Some of these I suspect will be
> considered false positives.  I think it might be worth splitting
> up the snprintf warning from -Wformat-length and adding a separate
> option to control it.

I must confess that I find these -Wformat-length warnings nearly
unparseable. They need much better documentation that explains where all
these seemingly random looking numbers are coming from.

And I also think that -Wformat-length should not be enabled by -Wall and
not even by -Wextra. The user should specifically ask for it.

-- 
Markus

Reply via email to