svn commit: trunk/busybox: coreutils include libbb loginutils

Peter Kjellerstedt peter.kjellerstedt at axis.com
Mon Jun 19 14:52:49 UTC 2006


> -----Original Message-----
> From: busybox-cvs-bounces at busybox.net 
> [mailto:busybox-cvs-bounces at busybox.net] On Behalf Of 
> landley at busybox.net
> Sent: Monday, June 19, 2006 01:59
> To: busybox-cvs at busybox.net
> Subject: svn commit: trunk/busybox: coreutils include libbb loginutils
> 
> Author: landley
> Date: 2006-06-18 16:59:03 -0700 (Sun, 18 Jun 2006)
> New Revision: 15421
> 
> Log:
> Undo all of the ugliness and some of the bloat from 15412.
> 
> Modified:
>    trunk/busybox/coreutils/stty.c
>    trunk/busybox/include/libbb.h
>    trunk/busybox/libbb/speed_table.c
>    trunk/busybox/loginutils/getty.c
> 
> Changeset:
> Modified: trunk/busybox/libbb/speed_table.c
> ===================================================================
> --- trunk/busybox/libbb/speed_table.c	2006-06-18 20:20:07 UTC (rev
15420)
> +++ trunk/busybox/libbb/speed_table.c	2006-06-18 23:59:03 UTC (rev
15421)
> @@ -4,160 +4,36 @@
>   *
>   * Copyright (C) 2003  Manuel Novoa III  <mjn3 at codepoet.org>
>   *
> - * This program is free software; you can redistribute it and/or
modify
> - * it under the terms of the GNU General Public License as published
by
> - * the Free Software Foundation; either version 2 of the License, or
> - * (at your option) any later version.
> - *
> - * This program is distributed in the hope that it will be useful,
> - * but WITHOUT ANY WARRANTY; without even the implied warranty of
> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> - * General Public License for more details.
> - *
> - * You should have received a copy of the GNU General Public License
> - * along with this program; if not, write to the Free Software
> - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
> - *
> + * Licensed under GPLv2 or later, see file LICENSE in this tarball
for details.
>   */
>  
> -#include <termios.h>
>  #include "libbb.h"
>  
> -struct speed_map {
> -	unsigned short speed;
> -	unsigned short value;
> +static const unsigned short speeds[] = {
> +	0, 50, 75, 110, 134, 150, 200, 300, 600, 1200, 1800, 2400, 4800,
9600,
> +   	19200, 38400, 57600>>8, 115200>>8, 230400>>8, 460800>>8,
500000>>8,
> +	576000>>8, 921600>>8, 1000000>>8, 1152000>>8, 1500000>>8,
2000000>>8,
> +	3000000>>8, 3500000>>8, 4000000>>8
>  };
>  
> -static const struct speed_map speeds[] = {
> -	{B0, 0},
> -	{B50, 50},
> -	{B75, 75},
> -	{B110, 110},
> -	{B134, 134},
> -	{B150, 150},
> -	{B200, 200},
> -	{B300, 300},
> -	{B600, 600},
> -	{B1200, 1200},
> -	{B1800, 1800},
> -	{B2400, 2400},
> -	{B4800, 4800},
> -	{B9600, 9600},
> -#ifdef B19200
> -	{B19200, 19200},
> -#elif defined(EXTA)
> -	{EXTA, 19200},
> -#endif
> -#ifdef B38400
> -	{B38400, 38400/256 + 0x8000U},
> -#elif defined(EXTB)
> -	{EXTB, 38400/256 + 0x8000U},
> -#endif
> -#ifdef B57600
> -	{B57600, 57600/256 + 0x8000U},
> -#endif
> -#ifdef B115200
> -	{B115200, 115200/256 + 0x8000U},
> -#endif
> -#ifdef B230400
> -	{B230400, 230400/256 + 0x8000U},
> -#endif
> -#ifdef B460800
> -	{B460800, 460800/256 + 0x8000U},
> -#endif
> -#ifdef B500000
> -	{B500000, 500000/256 + 0x8000U},
> -#endif
> -#ifdef B576000
> -	{B576000, 576000/256 + 0x8000U},
> -#endif
> -#ifdef B921600
> -	{B921600, 921600/256 + 0x8000U},
> -#endif
> -#ifdef B1000000
> -	{B1000000, 1000000/256 + 0x8000U},
> -#endif
> -#ifdef B1152000
> -	{B1152000, 1152000/256 + 0x8000U},
> -#endif
> -#ifdef B1500000
> -	{B1500000, 1500000/256 + 0x8000U},
> -#endif
> -#ifdef B2000000
> -	{B2000000, 2000000/256 + 0x8000U},
> -#endif
> -#ifdef B2500000
> -	{B2500000, 2500000/256 + 0x8000U},
> -#endif
> -#ifdef B3000000
> -	{B3000000, 3000000/256 + 0x8000U},
> -#endif
> -#ifdef B3500000
> -	{B3500000, 3500000/256 + 0x8000U},
> -#endif
> -#ifdef B4000000
> -	{B4000000, 4000000/256 + 0x8000U},
> -#endif
> -};
> -
> -enum { NUM_SPEEDS = (sizeof(speeds) / sizeof(struct speed_map)) };
> -
> -unsigned long bb_baud_to_value(speed_t speed)
> +unsigned int tty_baud_to_value(speed_t speed)
>  {
> -	int i = 0;
> +	int i;
>  
> -	do {
> -		if (speed == speeds[i].speed) {
> -			if (speeds[i].value & 0x8000U) {
> -				return ((unsigned long)
(speeds[i].value) & 0x7fffU) * 256;
> -			}
> -			return speeds[i].value;
> -		}
> -	} while (++i < NUM_SPEEDS);
> +	for (i=0; i<sizeof(speeds) / sizeof(*speeds); i++)
> +		if (speed == speeds[i] * (i>15 ? 256 : 1))
> +			return i>15 ? (i+4096-14) : i;
>  
>  	return 0;
>  }
>  
> -speed_t bb_value_to_baud(unsigned long value)
> +speed_t tty_value_to_baud(unsigned int value)
>  {
> -	int i = 0;
> +	int i;
>  
> -	do {
> -		if (value == bb_baud_to_value(speeds[i].speed)) {
> -			return speeds[i].speed;
> -		}
> -	} while (++i < NUM_SPEEDS);
> +	for (i=0; i<sizeof(speeds) / sizeof(*speeds); i++)
> +		if (value == (i>15 ? (i+4096-14) : i))
> +			return speeds[i] * (i>15 ? 256 : 1);
>  
> -	return (speed_t) - 1;
> +	return -1;
>  }

Nice idea, but totally wrong.

First of all the code above is incorrect.  Try to lookup 500000,
1000000, 1500000 etc using tty_baud_to_value().  It will not work
due to rounding errors.  This is however easily fixed by using a
factor of 400 rather than the currently used 256.  (The old code 
would also return an incorrect value for these speeds due to the 
same problem.)  This problem would have been very apparent for
anyone who had used the testcode which was also removed in the 
latest commit...

Second, and most importantly, the code above is complete crap for 
a lot of architectures that do not use the same baud rates as 
supported by i386.

The table was there for a reason, namely that it is the only way 
to get this right for all architectures.  And the use of a lot of
#ifdefs here is the only way to keep the size of the table down
at the same time as it works for all architectures.

I have attached a script and a table which shows the speeds, 
values and which architectures uses it (table generated using the 
include files from Linux 2.6.16).  The table is generated by
running:

grep 'define \+B[0-9]+' asm-*/termbits.h | termtable.pl

And as can be seen from this table, trying to turn this into code
is impossible due to the architectural differences...

I have also included a patch which reverts most of the previous
commit.  It also removes the hackery of using short rather than
long for the speed, since all this saves us is 16 bytes on i386
due to the increased complexity of the code and IMHO the 
maintainability of the code is more important than squeezing the
last byte out of it.  Then it adds all the missing speeds that 
were not present before.  And last it restores the test code since 
this is obviously needed, eventhough I guess the correct thing to 
do would be to add a testcase in the test suite.

//Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: termtable.pl
Type: application/octet-stream
Size: 331 bytes
Desc: termtable.pl
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060619/e3202172/attachment.obj 
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: termtable.txt
Url: http://lists.busybox.net/pipermail/busybox/attachments/20060619/e3202172/attachment.txt 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: speed_table.c.patch
Type: application/octet-stream
Size: 3491 bytes
Desc: speed_table.c.patch
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060619/e3202172/attachment-0001.obj 


More information about the busybox mailing list