This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCHv2] sysdeps/ieee754: prevent maybe-uninitialized errors [BZ #19444]


On Sat, 29 Sep 2018, Martin Jansa wrote:

> +/* With GCC 8 when compiling with -O the compiler
> +   warns that the variable 'temp',  may be used uninitialized.
> +   The switch above should cover all possible values of n & 3
> +   so I belive it's false possitive. */
> +DIAG_PUSH_NEEDS_COMMENT;
> +DIAG_IGNORE_NEEDS_COMMENT (8, "-Wmaybe-uninitialized");
>  	    b = invsqrtpi * temp / sqrtl (x);
> +DIAG_POP_NEEDS_COMMENT;

DIAG_* and associated comments should be indented to match the associated 
code rather than at the left margin.  Any alphabetic characters at the 
left margin in the middle of a function are a bad idea because they break 
things trying to determine the function name for diff context etc.

As per <https://sourceware.org/ml/libc-alpha/2018-09/msg00311.html> I 
think it would be best to add DIAG_IGNORE_O1_NEEDS_COMMENT to libc-diag.h, 
and use it for cases such as here that only need the diagnostics disabled 
for -O1.

glibc code is written and maintained by many people over time, so there 
shouldn't be an "I" in comments; they should reflect a current collective 
understanding rather than one person's view, and read as such.  Also, the 
point isn't that the switch *should* cover all possible value of n & 3, 
it's that it *does* cover all possible values (and sets temp in every 
case) and so the warning *is* a false positive.

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]