This is the mail archive of the ecos-discuss@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: RedBoot - Bug in _rb_gets()?


However, on closer inspection, I don't think that this really
makes much difference, at least not for RedBoot's main command
loop since it will never call _rb_gets() with a timeout larger
than the "idle" time.  It would possibly make a difference for
other routines which might want to use _rb_gets() with a very
large timeout, so I'll leave the change.

On Tue, 2001-10-16 at 08:34, Gary Thomas wrote:
> On Tue, 2001-10-16 at 02:16, Chris Zankel wrote:
> > Gary,
> > 
> > Sounds reasonable. But wouldn't that imply that the inner
> > loop is set to at maximum to 50 ms? Right now
> > mon_set_read_char_timeout() sets the timeout to whatever
> > you passed to it.
> > So, what happens right now is, if you do a mon_read_char_with_timeout()
> > with, for example, 1s timeout, that the inner-loop
> > mon_read_char_with_timout() waits 1s and the outer loop
> > loops 1000/50 = 20 times. The whole function waits 20s instead
> > of the specified one second.
> > 
> > Wouldn't it make more sense to rewrite that part to:
> > 
> > mon_set_read_char_timeout(timeout > 50? 50:timeout);
> > 
> > while (timeout > 0) {
> >   ...
> > }
> 
> Indeed, it does look like it's wrong.  I agree with your analysis and
> will make this change to the sources:
> 
> Index: redboot/current/src/io.c
> ===================================================================
> RCS file: /home/cvs/ecc/ecc/redboot/current/src/io.c,v
> retrieving revision 1.36
> diff -u -5 -p -r1.36 io.c
> --- redboot/current/src/io.c	23 Aug 2001 21:27:13 -0000	1.36
> +++ redboot/current/src/io.c	15 Oct 2001 23:24:00 -0000
> @@ -295,19 +295,20 @@ _rb_gets(char *buf, int buflen, int time
>          if (getc_script(&c))
>              do_idle(false);
>          else
>  #endif
>          if ((timeout > 0) && (ptr == buf)) {
> -            mon_set_read_char_timeout(timeout);
> +#define MIN_TIMEOUT 50
> +            mon_set_read_char_timeout(timeout > MIN_TIMEOUT ? MIN_TIMEOUT : timeout);
>              while (timeout > 0) {
>                  res = mon_read_char_with_timeout(&c);
>                  if (res) {
>                      // Got a character
>                      do_idle(false);
>                      break;
>                  }
> -                timeout -= 50;
> +                timeout -= MIN_TIMEOUT;
>              }
>              if (res == false) {
>                  do_idle(true);
>                  return _GETS_TIMEOUT;  // Input timed out
>              }
> > 
> > 
> > Thanks,
> > Chris
> > 
> > Gary Thomas wrote:
> > 
> > >>Although the timeout value for the mon_read_char_with_timeout()
> > >>function has already been set to a <timeout> value the while
> > >>loop also loops over this <timeout> variable:
> > >>
> > >>...
> > >>    mon_set_read_char_timeout(timeout);
> > >>
> > >>    while (timeout > 0) {
> > >>       res = mon_read_char_with_timeout(&c);
> > >>       if (res) {	...      }
> > >>       timeout -= 50;
> > >>    }
> > >>...
> > >>
> > > 
> > > There are two separate "timeout"s at work here.  The innermost one is
> > > what is used by the read_char function to decide if a character has
> > > arrived within some period of time.  The outermost is used by RedBoot
> > > to allow it to do some things while waiting for characters to arrive
> > > and will be much larger (orders of magnitude) than the innermost
> > > timeout.  The idea is for RedBoot to see if a character has arrived
> > > or wait at most the "innermost" time.  If no character has arrived, then
> > > RedBoot can take on some "background" processing, e.g. checking for
> > > any arriving network connections, blanking LCD screens, etc.
> > > 
> > 



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]