svn commit: trunk/busybox: coreutils include libbb loginutils

Tue Jun 20 09:14:50 PDT 2006

On Monday 19 June 2006 10:52 am, Peter Kjellerstedt wrote:
> > -----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.

*shrug*  I'd really rather spend my time on other areas and not look at this 
bit, but I've learned that if I don't pay attention to vapier's commits we 
wind up with things like the e2fsprogs directory.

> 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.

Sigh.  I suspected that.  (I was hoping it wasn't the case, but...)

> 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.

Why was vapier's last patch submitted at all?  Why do we need to support 
serial ports going greater than 400kbps?  I thought the hardware didn't do 
that.  Is there a big enough need for us to _care_ about this?

> I have also included a patch which reverts most of the previous
> commit.

I'd happily revert the last _two_ patches to this.  Why is this particular bit 
of feature creep necessary?  (I suspect the reason I'm being so hard on it is 
nobody ever said why it was needed before just slapping more bloat onto the 
tree...)

And yes, the test suite is good.  Having random test you can't automate 
regression finding with is pretty much useless.

Rob
-- 
Never bet against the cheap plastic solution.