On Wednesday 12 November 2008 20:35, Cathey, Jim wrote:
> Both approaches seem to work just fine. The add_iacs()
> patch I did is smaller, and I believe fits better into
> the existing structure of telnetd, such as it is:
>
> $ size networking/telnetd.o
> text data bss dec hex filename
> 3752 16 16 3784 ec8 networking/telnetd.o
> $ # Replace add_iacs() patch with iac_safe_write() patch & rebuild...
> $ size networking/telnetd.o
> text data bss dec hex filename
> 3792 16 16 3824 ef0 networking/telnetd.o
> $
>
> (These are mips64 numbers.)
>
> I had to apply iac_safe_write() code as a patch to what we
> had, replacing my patch. (For comparison purposes.) I couldn't
> get the current svn to fully build for me, and I've got nearly
> zero time to spend on this. I believe the comparative results
> are still valid, however.
>
> Essentially both patches each apply one new subroutine, which
> are listed here for easy comparison:
>
> static void
> add_iacs(struct tsession *ts, int *pnum_fromtty)
> {
> unsigned char *ptr = TS_BUF2 + ts->rdidx2;
> unsigned char *end = ptr + MIN(BUFSIZE - ts->rdidx2, *pnum_fromtty);
Let's assume there are full "BUFSIZE - ts->rdidx2" bytes
in the buffer, and the very first byte is 0xff.
> while (ptr < end) {
> if (*ptr == IAC) {
> memmove(ptr+1, ptr, end - ptr);// Results in dupe IAC!
Here we would store one byte past the buffer's end.
> if (end < TS_BUF2 + BUFSIZE)
> end++;
...else - just forget that pesky last byte, who cares?
> (*pnum_fromtty)++;
> ptr++; // Don't process the dupe next time either.
> }
> ptr++;
> }
> }
>
> *** Versus ***
>
> static size_t iac_safe_write(int fd, const char *buf, size_t count)
> {
> const char *oxff;
> size_t wr, rc, total;
>
> total = 0;
> while (1) {
> if (count == 0)
> return total;
> if (*buf == (char)0xff) {
> rc = safe_write(fd, "\xff\xff", 2);
> if (rc != 2)
> break;
> buf++;
> total++;
> count--;
> continue;
> }
> /* count != 0, *buf != 0xff */
> oxff = memchr(buf, 0xff, count);
> wr = count;
> if (oxff)
> wr = oxff - buf;
> rc = safe_write(fd, buf, wr);
> if (rc != wr)
> break;
> buf += rc;
> total += rc;
> count -= rc;
> }
> /* here: rc - result of last short write */
> if ((ssize_t)rc < 0) { /* error? */
> if (total == 0)
> return rc;
> rc = 0;
> }
> return total + rc;
> }
>
> I happen to think that mine is simpler; it's for certain smaller!
I am all for smaller, as long as it does not introduce bugs.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
