Re: CVS commit: src/lib/libarch/alpha
On Thu, Mar 22, 2012 at 09:35:48AM -0600, Warner Losh wrote: > When was the last time that NetBSD could be compiled with a K&R compiler? > 1995? Dunno, but we killed supported ports because we have no working C99 compiler for them (playstation2). Martin
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 K&R (!) 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 K&R compiler? 1995? Warner
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 K&R (!) is simply wrong. ISO C90 is now 22 years old. Traditional C is irrelevant. Joerg
Re: CVS commit: src/lib/libarch/alpha
In article <20120322100642.ga1...@apb-laptoy.apb.alt.za>, Alan Barrett 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 K&R 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 08:54:49AM +, Havard Eidnes wrote: >> Module Name: src >> Committed By:he >> Date:Thu Mar 22 08:54:48 UTC 2012 >> >> 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. > > Again, why is this needed or desirable? Both sides of the operator are > unsigned, so in the worst case, promition happens to the wider of the > type and the result is truncated immediately. The casts are yet again > just making the code uglier. The comment in lint says: /* * ANSI C does not perform balancing for shift operations, * but traditional C does. If the width of the right operand * is greather than the width of the left operand, than in * traditional C the left operand would be extendet to the * width of the right operand. For SHL this may result in * different results. */ and the corresponding warning from lint is semantics of '<<' change in ANSI C; use explicit cast So that's what I did. If you think lint is wrong, I encourage you to improve it. 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. Regards, - Håvard
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/libarch/alpha
On Thu, Mar 22, 2012 at 08:54:49AM +, Havard Eidnes wrote: > Module Name: src > Committed By: he > Date: Thu Mar 22 08:54:48 UTC 2012 > > 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. Again, why is this needed or desirable? Both sides of the operator are unsigned, so in the worst case, promition happens to the wider of the type and the result is truncated immediately. The casts are yet again just making the code uglier. Joerg
