Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/lib/libc/locale



> Date: Sat, 17 Aug 2024 17:55:28 -0400
> From: Christos Zoulas <christos%zoulas.com@localhost>
> 
> There is only one bug. I have sent mail to Roland about it and will revert once it is fixed.
> 
> The rest are:
> - cast through (void *) 

There is no need to cast through (void *) -- not in C, not in KNF, and
not for any other reason I know of.  It's not a big deal to leave in,
but it's also unnecessary clutter and I would rather remove it.

> - add casts because __BITS returns uintmax_t which is wider than the destination types.

I'm sympathetic to the proposition that implicit integer conversions
can conceal scary bugs in C.  But in this case, the output is
trivially guaranteed to lie within the desired range, so it is obvious
that the integer conversion never loses anything even though the
intermediate types are larger -- that's a big part of the point of the
convenience and legibility of __BITS and __SHIFTIN/__SHIFTOUT.

If lint complains because it can't figure this out, that's a lint bug.
And the casts make the code much harder to read.  Please revert this
and let's make lint serve our needs, not make us serve lint's needs.

		if (pc8)
			*pc8 = 0xf0 | __SHIFTOUT(c32, __BITS(20,18));
		S->buf[0] = 0x80 | __SHIFTOUT(c32, __BITS(17,12));
		S->buf[1] = 0x80 | __SHIFTOUT(c32, __BITS(11,6));
		S->buf[2] = 0x80 | __SHIFTOUT(c32, __BITS(5,0));

versus

		if (pc8)
			*pc8 = (char8_t)(
			    0xf0 | __SHIFTOUT(c32, __BITS(20,18)));
		S->buf[0] = (char8_t)(
		    0x80 | __SHIFTOUT(c32, __BITS(17,12)));
		S->buf[1] = (char8_t)(
		    0x80 | __SHIFTOUT(c32, __BITS(11,6)));
		S->buf[2] = (char8_t)(
		    0x80 | __SHIFTOUT(c32, __BITS(5,0)));


Home | Main Index | Thread Index | Old Index