On Sat, 29 Nov 2014, Takahashi Yoshihiro wrote:
Log:
MFi386: r275059, r275061, r275062 and r275191 (by rdivacky)
Shrink boot2 by a couple more bytes.
These changes don't look too good to me. One is just a bug.
Modified: head/sys/boot/pc98/boot2/boot2.c
==============================================================================
--- head/sys/boot/pc98/boot2/boot2.c Sat Nov 29 11:50:19 2014
(r275240)
+++ head/sys/boot/pc98/boot2/boot2.c Sat Nov 29 12:22:31 2014
(r275241)
...
@@ -533,6 +534,7 @@ parse()
const char *cp;
unsigned int drv;
int c, i, j;
+ size_t k;
This still has a style bug (unsorting).
while ((c = *arg++)) {
if (c == ' ' || c == '\t' || c == '\n')
@@ -555,7 +557,7 @@ parse()
#if SERIAL
} else if (c == 'S') {
j = 0;
- while ((unsigned int)(i = *arg++ - '0') <= 9)
+ while ((i = *arg++ - '0') <= 9)
j = j * 10 + i;
if (j > 0 && i == -'0') {
comspeed = j;
This used to work. The cast was an obfuscated way of checking for
characters less than '0'. Removing the cast breaks the code. Garbage
non-digits below '0' (including all characters below '\0') are now
treated as digits.
This still asks for the pessimization of promoting *arg++ to int before
checking it. The compiler might be able to unpessimize this, but large
code shouldn't be asked for. The following should be better:
uint8_t uc;
...
/* Check that the compiler generates 8-bit ops. */
while ((uc = *arg++ - '0') <= 9)
j = j * 10 + uc;
if (j > 0 && uc == (uint8_t)-'0') {
comspeed = j;
I don't see a correct way to avoid the second check of uc.
Subtracting '0' and using the unsigned obfuscation to check for digits
is used in several other places:
drv = *arg - '0';
if (drv > 9)
...;
This works since 'drv' is unsigned. Drive numbers above 9 are not supported,
so uint8_t could be used here, saving 1 byte. At least 1 more byte can be
saved in each of 'drv == -1' and 'if (drv == -1)'. 'drv' would probably
need to be promoted before use; it is only used once to initialize
dsk.drive. It can be promoted there using no extra bytes by statically
initializing dsk.drive to 0 and storing only 8 bits from 'drv'. The total
savings should be 4-5 bytes.
dsk.unit = *arg - '0';
if (arg[1] != ',' || dsk.unit > 9)
...;
dsk.unit is also unsigned. Unit numbers above 9 are not supported, so
uint8_t should work here too, but since dsk.unit probably needs to be
a 32-bit type and there is no local variable like 'drv', the savings are
not as easy.
dsk.slice = WHOLE_DISK_SLICE;
...
dsk.slice = *arg - '0' + 1;
if (dsk.slice > NDOSPART + 1)
...;
Now disk.slice is already uint8_t, so this code should be optimal. However,
when dsk.slice is used it has to be promoted and about 3 bytes are wasted
there. To avoid this wastage, make disk.slice plain int (no need for
unsigned hacks), initialize it all to 0, and store only the low 8 bits
in the above.
There are so many of these that it might be smaller to use a function
that handles the general case. I suspect that this is not in fact smaller,
since it would need complications like passing back the endptr (like
strtol(), not atoi()).
Bruce
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"
