This is the mail archive of the
ecos-discuss@sources.redhat.com
mailing list for the eCos project.
Re: RedBoot - Bug in _rb_gets()?
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.
> >
>