On Sun, Feb 14, 2010 at 10:47 PM, John Denker <j...@av8n.com> wrote:
> The following commit message should be self-explanatory:
>
> commit 224ce694fa8ba7dede0e413b81e5dd52e5e65f15
> Author: John Denker <j...@av8n.com>
> Date: Thu Feb 11 21:13:19 2010 -0700
>
> Problem was: readline writes out-of-bounds, corrupts memory.
> Problem was: readline seeks on files that don't support seek.
> Problem was: readline fails to detect seek errors, returns garbage.
> Problem was: readline wildly inefficient, re-reading same data
> again and again.
>
> Add utility to test for read/write bugs.
> Replace readline with a version that is more compact, more
> maintainable, more extensible, more correct, more efficient, and
> able to read from named FIFOs and other things that don't seek.
>
>
> For details (476 lines worth) see:
> http://www.av8n.com/fly/fgfs/readline.patch
>
I took a look at your patch and have a couple of comments. It's true that
readline() is pretty gross; parts of it were written by yours truly. Some of
the grossness is due to a hack which lets a file be treated as an infinitely
repeating stream of bytes, very convenient for demos at SIGGRAPH. Your patch
breaks that hack. I won't argue too strongly that the hack belongs in
SGFile, but I want to have some story for replacing it, possibly in
FGGeneric::process(), before we blow it away.
Having done something like this recently in another project, I think the
code code be more clear with some use of std::string and the STL. Here's a
sketch:
if (length == 0)
throw(...);
while (!eof_flag) {
string::size_type delimPos = _buffer.find(delim);
if (delimPos != string::npos) {
size_t bytesCopied = std::max(length - 1, delimPos - 1);
_buffer.copy(buf, bytesCopied);
buf[bytesCopied] = 0;
_buffer.erase(0, bytesCopied + 1);
return delimPos;
}
char readBuf[256];
int readRet = ::read(fp, readBuf, sizeof(readBuf));
if (readRet < 0)
throw(...);
else if (readRet == 0)
eof_flag = true;
else
_buffer.append(readBuf, readRet);
}
return 0;
>
> ======================================================
>
> The code in sg_file.cxx appears to run parallel to code
> in other places such as sg_socket.cxx I wouldn't be
> surprised if the other places needed fixing, too ...
> but I haven't looked.
>
Tim
------------------------------------------------------------------------------
SOLARIS 10 is the OS for Data Centers - provides features such as DTrace,
Predictive Self Healing and Award Winning ZFS. Get Solaris 10 NOW
http://p.sf.net/sfu/solaris-dev2dev
_______________________________________________
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel
