Re: CVS commit: src/lib/libc/arch/alpha/gen
On Wed, Mar 21, 2012 at 11:57:25PM +0100, Nicolas Joly wrote: On Wed, Mar 21, 2012 at 10:42:58PM +0200, Alan Barrett wrote: On Wed, 21 Mar 2012, Havard Eidnes wrote: Modified Files: src/lib/libc/arch/alpha/gen: fpgetround.c fpsetround.c Log Message: Add some casts to get rid of bitwise op on signed value is non-portable warning from lint. I see no bitwise ops on signed values here. - return ((fpcrval.u64 58) 0x3); + return ((fp_rnd)(fpcrval.u64 58) 0x3); fpcrval.u64 is uint64_t. After the integer promotions, it's still uint64_t (unless that's smaller than int, which is not the case for any existing NetBSD port). After 58, it's still uint64_t. 0x3 is a signed int, but the usual arithmetic conversions should convert it to uint64_t. The commit message reference the wrong lint warning. /local/src/NetBSD/src/lib/libc/arch/alpha/gen/fpgetround.c(61): warning: conversion from 'unsigned long' to 'enum unnamed' may lose accuracy [132] /local/src/NetBSD/src/lib/libc/arch/alpha/gen/fpsetround.c(61): warning: conversion from 'unsigned long' to 'enum unnamed' may lose accuracy [132] Which is bogus because of the ' 3' which brings the value inside valid range. The cast is really in the wrong place as well. I am 100% against adding casts of numeric values to appease a tool that isn't tracking the domains of the expressions. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/lib/libarch/alpha
On Thu, 22 Mar 2012, Havard Eidnes wrote: Modified Files: src/lib/libarch/alpha: alpha_pci_io.c Log Message: Add a cast of the shift count to int32_t, so that we don't try to do int32_t long, since ANSI C doesn't perform balancing before the shift operation according to lint. Should not make a difference, offset is limited to 0..3 anyway. I don't know what balancing means, but this seems bogus to me. The type of the right hand operand of the operator is irrelevant; only its value is important. (See sectiopn 6.5.7 of the C99 standard.) I think it's fine to add casts that are not really nbecessary, if they improve the readability or portability of the code. The cast here does not do that, and I think it should not be added. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/arch/alpha/gen
On Thu, Mar 22, 2012 at 07:27:18 +, David Laight wrote: On Wed, Mar 21, 2012 at 11:57:25PM +0100, Nicolas Joly wrote: On Wed, Mar 21, 2012 at 10:42:58PM +0200, Alan Barrett wrote: On Wed, 21 Mar 2012, Havard Eidnes wrote: Modified Files: src/lib/libc/arch/alpha/gen: fpgetround.c fpsetround.c Log Message: Add some casts to get rid of bitwise op on signed value is non-portable warning from lint. I see no bitwise ops on signed values here. -return ((fpcrval.u64 58) 0x3); +return ((fp_rnd)(fpcrval.u64 58) 0x3); fpcrval.u64 is uint64_t. After the integer promotions, it's still uint64_t (unless that's smaller than int, which is not the case for any existing NetBSD port). After 58, it's still uint64_t. 0x3 is a signed int, but the usual arithmetic conversions should convert it to uint64_t. The commit message reference the wrong lint warning. /local/src/NetBSD/src/lib/libc/arch/alpha/gen/fpgetround.c(61): warning: conversion from 'unsigned long' to 'enum unnamed' may lose accuracy [132] /local/src/NetBSD/src/lib/libc/arch/alpha/gen/fpsetround.c(61): warning: conversion from 'unsigned long' to 'enum unnamed' may lose accuracy [132] Which is bogus because of the ' 3' which brings the value inside valid range. The cast is really in the wrong place as well. I am 100% against adding casts of numeric values to appease a tool that isn't tracking the domains of the expressions. Me too. Code becomes very hard to read when bit dances in complex expressions are hidden behind casts and in some cases all of that is sliced into a word-per-line word salat to fit into 80 columns (b/c the cast now consumes more than half of the available line length). -uwe
Re: CVS commit: src/lib/libc/gen
On Thu, 22 Mar 2012, Havard Eidnes wrote: Modified Files: src/lib/libc/gen: modf_ieee754.c Log Message: Add a pair of casts to silence lint about conversion possibly losing bits. - v.dblu_dbl.dbl_fracl = frac 0x; - v.dblu_dbl.dbl_frach = frac 32; + v.dblu_dbl.dbl_fracl = (u_int) (frac 0xULL); + v.dblu_dbl.dbl_frach = (u_int) (frac 32); This looks like another bogus lint warning. Because of the shifts and masks, the values are guaranteed to fit in 32 bits. Please could we stop adding casts to appease broken warnings. --apb (Alan Barrett)
Re: CVS commit: src/lib/libarch/alpha
In article 20120322100642.ga1...@apb-laptoy.apb.alt.za, Alan Barrett a...@cequrux.com wrote: I don't know what balancing means, but this seems bogus to me. The type of the right hand operand of the operator is irrelevant; only its value is important. (See sectiopn 6.5.7 of the C99 standard.) Balancing means that in KR c the type of the result of the shift operation was the wider of the types of the two shift operands. christos
Re: CVS commit: src/lib/libarch/alpha
On Thu, Mar 22, 2012 at 02:51:08PM +0100, Havard Eidnes wrote: IMHO, as long as lint is capable of helping us spot actual problems, adding a few of these sorts of constrcucts seems like a small price to pay. It doesn't. From what I see, the signal to noise ratio of lint is completely inacceptable and for that very reason, uglifying the code with questionable constructs is not acceptable. Even worse, changing code for undefined/misdefined behavior of KR (!) is simply wrong. ISO C90 is now 22 years old. Traditional C is irrelevant. Joerg
Re: CVS commit: src/lib/libarch/alpha
On Mar 22, 2012, at 8:43 AM, Joerg Sonnenberger wrote: On Thu, Mar 22, 2012 at 02:51:08PM +0100, Havard Eidnes wrote: IMHO, as long as lint is capable of helping us spot actual problems, adding a few of these sorts of constrcucts seems like a small price to pay. It doesn't. From what I see, the signal to noise ratio of lint is completely inacceptable and for that very reason, uglifying the code with questionable constructs is not acceptable. Even worse, changing code for undefined/misdefined behavior of KR (!) is simply wrong. ISO C90 is now 22 years old. Traditional C is irrelevant. When was the last time that NetBSD could be compiled with a KR compiler? 1995? Warner
