Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

I see we're talking past each other on some points and are in violent
agreement on others, probably because the discussion overheated.

On Tue, Mar 2, 2010 at 11:20 AM, John Denker  wrote:

> On 03/02/2010 01:08 AM, Tim Moore wrote:
>
> > Furthermore, I can't parse the "suspend development" comment. It is
> coming
> > from some alternate reality of git usage.
>
> We are definitely talking about two different realities.
>
> >  First off, I did identify the
> > commit id where you made changes to use getline:  d5e6aa6235f7.
>
> That's a commit to the "FG source tree".  Until that hex
> number was mentioned, I thought we were talking about the
> code over in the simgear tree.
>
Sorry, I thought I made that clear when I said:

> I'm looking at io/sg_file.cxx in the sport branch. I see the old
> implementation of readline inside an
> "execrable_readline" #ifdef. I don't see any other implementation of
> readline.
>
> I.e., I've checkout out the sport branch of simgear and have looked at your
changes there.

> Perhaps my question would go away if I fetched your sport flightgear repo
> too; since I don't have a commit id for the patch where you presumably
> changed "readline" to "getline", I haven't. But it seems to me that you
> could easily implement the old "execrable" readline interface in terms of
> your spiffy getline and save yourself the trouble of changing "readline"
> calls to "getline."

 I.e., I'm now looking for where you replaced uses of "readline" with
"getline." I thought it was clear that I was looking in the flightgear repo.

This process isn't made any easier by having separate flightgear and simgear
repositories, and unfortunately that's not going to change anytime soon.

>
> > There aren't
> > any more yet. You could have just told me that!
>
> You could have asked!
>
> If "not any more yet" is the answer, I still don't know
> what the question is (or was).
>
> > At the start of this very thread
> > you sent us the commit id of a change you wished to share and have
> committed
> > to the sources. In the specific case of getline changes to flightgear,
> you
> > have so far only made one commit which uses getline. How hard is it to
> send
> > us that?
>
> It's not hard.  If you want me to send it, all you have to
> do is ask.  But I'm pretty sure that's not what you want ...
> and I still don't know what you do want.
>
It is what I wanted, but I have it now :)

>
> I don't routinely send patches to this list, for multiple
> reasons.  One reason is the fact that most list-members are
> not interested in the details.  I assume anybody who wants
> the details will ask ... or look at the git-logs ... or
> whatever.
>
I think patches are completely appropriate to send to the list. If they are
in unified diff form, it is very easy to do  a quick review and see if the
patch is interesting, on the right track, well written, etc. Saves me the
trouble of asking, looking in logs, etc.

>
> Another reason is the fact that at the time of the first
> announcement it is quite impossible to know what further
> developments will take place.  As of today I can answer a
> specific question by saying "not any more yet" or some such,
> but it is impossible in principle to answer such questions
> in advance.
>
> You are suggesting something that should be committed in some specific
form, right?

> Also, when I am told "the code looks OK" followed by
> suggestions for further developments, it is hard for
> me to translate that into a question that could be
> answered by saying "not any more yet" or into a wish
> that could be granted by emailing a patch.
>
> I agree that my commentary was a bit vague. But it looks like that you in
fact did further work on readline/getline...

> >  If you're saying that you might make several commits
> > in implementing a change and can't identify a specific commit that
> contains
> > it, then you need to reconsider how you're working in light of git
> workflow.
>
> That's not even remotely what I'm saying.  I know how to
> make git branches.  I grow a lot more branches than I
> push to gitorious.  I suspect you do too.
>
> > I'm not going to look at the global state of your repo at a
> > point in time and tease out the changes related to some feature you'd
> like
> > merged.
>
> I understand that.  Really I do.
>
> Now, it's your turn.  You need to understand that I cannot
> continually rebase eleventeen different bits of work so that
> "X" is sitting on top of the tree, on the off-chance that
> somebody will want to look at "X" today ... without any way
> for me to know what "X" is.
>
Of course not! That's why I was asking for a specific commit to look at in
your sport repo. Because your sport repos are gitorious clones of the repos
I maintain, it's easy for me work with specific commits or branches, rebase
them at *my* leisure, modify them, etc. It's true that a patch becomes less
interesting as the files it affects change upstream and it requires more and
more effort to merge, but

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On 03/02/2010 01:08 AM, Tim Moore wrote:

> Furthermore, I can't parse the "suspend development" comment. It is coming
> from some alternate reality of git usage. 

We are definitely talking about two different realities.

>  First off, I did identify the
> commit id where you made changes to use getline:  d5e6aa6235f7.

That's a commit to the "FG source tree".  Until that hex
number was mentioned, I thought we were talking about the 
code over in the simgear tree.

> There aren't
> any more yet. You could have just told me that!

You could have asked!

If "not any more yet" is the answer, I still don't know 
what the question is (or was).

> At the start of this very thread
> you sent us the commit id of a change you wished to share and have committed
> to the sources. In the specific case of getline changes to flightgear, you
> have so far only made one commit which uses getline. How hard is it to send
> us that?

It's not hard.  If you want me to send it, all you have to 
do is ask.  But I'm pretty sure that's not what you want ... 
and I still don't know what you do want.

I don't routinely send patches to this list, for multiple
reasons.  One reason is the fact that most list-members are 
not interested in the details.  I assume anybody who wants 
the details will ask ... or look at the git-logs ... or 
whatever.

Another reason is the fact that at the time of the first 
announcement it is quite impossible to know what further 
developments will take place.  As of today I can answer a
specific question by saying "not any more yet" or some such,
but it is impossible in principle to answer such questions 
in advance.

Also, when I am told "the code looks OK" followed by
suggestions for further developments, it is hard for
me to translate that into a question that could be
answered by saying "not any more yet" or into a wish 
that could be granted by emailing a patch.

>  If you're saying that you might make several commits
> in implementing a change and can't identify a specific commit that contains
> it, then you need to reconsider how you're working in light of git workflow.

That's not even remotely what I'm saying.  I know how to
make git branches.  I grow a lot more branches than I 
push to gitorious.  I suspect you do too.

> I'm not going to look at the global state of your repo at a
> point in time and tease out the changes related to some feature you'd like
> merged.

I understand that.  Really I do.

Now, it's your turn.  You need to understand that I cannot
continually rebase eleventeen different bits of work so that
"X" is sitting on top of the tree, on the off-chance that
somebody will want to look at "X" today ... without any way
for me to know what "X" is.

If you want cooperation, all you need to do is ask.  For
example, if you want me to put "X" into a special branch and 
push it somewhere, let me know what "X" is and I'll see what 
I can do.


--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Tue, Mar 2, 2010 at 12:47 AM, John Denker  wrote:

> On 03/01/2010 04:13 PM, Tim Moore wrote:
>
> > I'm looking at io/sg_file.cxx in the sport branch. I see the old
> > implementation of readline inside an
> > "execrable_readline" #ifdef. I don't see any other implementation of
> > readline.
>
> > Perhaps my question would go away if I fetched your sport flightgear repo
> > too;
>
> You could fetch it into a branch of whatever you're
> already using.  Gitorious makes this easy and efficient.
>

> > since I don't have a commit id for the patch where you presumably
> > changed "readline" to "getline", I haven't.
>
> In such a branch, you could do this:
>  git log --oneline --author=jsd sg_file.cxx
>
Yes, I did something similar to look at your implementation of getline in
simgear. For your uses of getline in flightgear I don't know a priori in
which files they might occur in flightgear. And yes, I'm quite familiar with
"git log", having used it to pick apart changes in your private sportmodel
repo, among other things. I know how to do "git log -Sgetline sport/sport",
but that is really besides the point.

>
> It will tell you all three commit-ids.  There is no need
> to "grovel through logs".
>
> I consider that groveling through logs.

> There will in general not be any such thing as "the"
> commit-id because I cannot suspend development while
> waiting for somebody to get interested in this-or-that
> bit of work I did.
>
This comment is wrong on so many levels. First off, I did identify the
commit id where you made changes to use getline:  d5e6aa6235f7. There aren't
any more yet. You could have just told me that! It's not productive to make
me look at it myself when I'm taking time late at night to try to review
your changes.

Furthermore, I can't parse the "suspend development" comment. It is coming
from some alternate reality of git usage. At the start of this very thread
you sent us the commit id of a change you wished to share and have committed
to the sources. In the specific case of getline changes to flightgear, you
have so far only made one commit which uses getline. How hard is it to send
us that? More generally, no one expects you to sit around while I/we
consider your patches. If you're saying that you might make several commits
in implementing a change and can't identify a specific commit that contains
it, then you need to reconsider how you're working in light of git workflow.
Put these changes on a branch! I keep several branches going here that
contain specific features I'm working on. This makes it trivial to submit /
commit them when the time is right without dragging in the rest of the work.

>
> The track record indicates that most of the FG code I
> write eventually gets incorporated into the "official"
> repository, but it is delayed months or years.
>
> No wonder. I'm not going to look at the global state of your repo at a
point in time and tease out the changes related to some feature you'd like
merged. I'm all too familiar with the git tools that can be used to do that,
but I really have better things to do.

Anyway, back to getline. In looking at d5e6aa6235f7, it seems that there
some unrelated changes for bidirectional i/o. Is that correct?

> > But it seems to me that you
> > could easily implement the old "execrable" readline interface in terms of
> > your spiffy getline and save yourself the trouble of changing "readline"
> > calls to "getline."
>
> I don't see an easy way of reimplementing readline _per se_
> in terms of ::getline or SGFile::getline.  The semantics
> are too different, especially for small buffers;  getline
> might hang in cases where readline would not, and dealing
> with that would be more work than I'm being paid to do.
>
>
OK, my suggestion was probably gratuitous. I'm not sure about this
difference in getline behavior; presumably a "line too long" error is a
local problem with the protocol and not some sort of network attack. The

istream& getline (char* s, streamsize n, char delim )

version of getline might be better here.

 It is probably true that any code that uses readline
> would be improved by switching to the getline semantics,
> but again, finding all uses of readline, understanding
> them, and switching them over is more work than I'm
> being paid to do.
>
> I thought you had done that, but no problem. Reviewing your patches is more
work than I'm being paid to do, BTW.


> Don't let the perfect be the enemy of the good.
>
> Ha!

Tim
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On 03/01/2010 04:13 PM, Tim Moore wrote:

> I'm looking at io/sg_file.cxx in the sport branch. I see the old
> implementation of readline inside an
> "execrable_readline" #ifdef. I don't see any other implementation of
> readline.

> Perhaps my question would go away if I fetched your sport flightgear repo
> too; 

You could fetch it into a branch of whatever you're
already using.  Gitorious makes this easy and efficient.

> since I don't have a commit id for the patch where you presumably
> changed "readline" to "getline", I haven't. 

In such a branch, you could do this:
  git log --oneline --author=jsd sg_file.cxx

It will tell you all three commit-ids.  There is no need
to "grovel through logs".

There will in general not be any such thing as "the"
commit-id because I cannot suspend development while 
waiting for somebody to get interested in this-or-that 
bit of work I did.

The track record indicates that most of the FG code I 
write eventually gets incorporated into the "official"
repository, but it is delayed months or years.

> But it seems to me that you
> could easily implement the old "execrable" readline interface in terms of
> your spiffy getline and save yourself the trouble of changing "readline"
> calls to "getline."

I don't see an easy way of reimplementing readline _per se_
in terms of ::getline or SGFile::getline.  The semantics
are too different, especially for small buffers;  getline
might hang in cases where readline would not, and dealing
with that would be more work than I'm being paid to do.

It is probably true that any code that uses readline 
would be improved by switching to the getline semantics,
but again, finding all uses of readline, understanding
them, and switching them over is more work than I'm
being paid to do.

Don't let the perfect be the enemy of the good.

--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Tue, Mar 2, 2010 at 12:01 AM, John Denker  wrote:

> On 03/01/2010 03:56 PM, Tim Moore wrote:
>
> > getline looks fine.
>
> :-)
>
> > Instead of getting steamed about readline, why not
> > implement it in terms of getline?
>
> I did.
>
> If this is really a question, please clarify the question.
>
I'm looking at io/sg_file.cxx in the sport branch. I see the old
implementation of readline inside an
"execrable_readline" #ifdef. I don't see any other implementation of
readline.

Perhaps my question would go away if I fetched your sport flightgear repo
too; since I don't have a commit id for the patch where you presumably
changed "readline" to "getline", I haven't. But it seems to me that you
could easily implement the old "execrable" readline interface in terms of
your spiffy getline and save yourself the trouble of changing "readline"
calls to "getline."

Tim
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On 03/01/2010 03:56 PM, Tim Moore wrote:

> getline looks fine. 

:-)

> Instead of getting steamed about readline, why not
> implement it in terms of getline?

I did.

If this is really a question, please clarify the question.

--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Tue, Feb 23, 2010 at 11:36 PM, John Denker  wrote:

> On 02/15/2010 03:19 AM, Tim Moore in part wrote:
>
> > readline() is pretty gross;
>
> The best way to remove the grossness is to extirpate
> readline and replace it with something that has a
> nicer interface ... such as returning a std::string.
>
> I wrote a getline function to do this.  Much cleaner.
> No need for 16kbyte buffers all over the place.  (*)
>
> The patch can be found in the usual place:
>   
> http://gitorious.org/~jsd/fg/sport-simgear/commits/sport
>
> An aside, it's easier for me to find a commit that has a commit ID. In this
case I guessed what file your getline is in and used git log to find the
patch, but in general I don't want to grovel through logs.

> Over on the FG side, this requires some minor upgrades:
>   
> http://gitorious.org/~jsd/fg/sport-model/commits/sport
>
> I tried asking for suggestions and/or review off-list,
> but it appears that mail to timoor...@gmail.com never
> goes through.  Is it a list-mail address only?
>
> In an ideal world you would receive more feedback from a list than an
individual. But anyway...

getline looks fine. Instead of getting steamed about readline, why not
implement it in terms of getline?

Tim
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Tue, Feb 23, 2010 at 11:36 PM, John Denker  wrote:

>
> I tried asking for suggestions and/or review off-list,
> but it appears that mail to timoor...@gmail.com never
> goes through.  Is it a list-mail address only?
>
No. Sometimes I'm busier and/or lamer than other times. I did get your mail
on sg_file and am on the verge of checking in what you submitted, but
haven't quite found the time to give it the attention it deserves.

Tim
--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On 02/15/2010 03:19 AM, Tim Moore in part wrote:

> readline() is pretty gross;

The best way to remove the grossness is to extirpate
readline and replace it with something that has a 
nicer interface ... such as returning a std::string.

I wrote a getline function to do this.  Much cleaner.
No need for 16kbyte buffers all over the place.  (*)

The patch can be found in the usual place:
  http://gitorious.org/~jsd/fg/sport-simgear/commits/sport

Over on the FG side, this requires some minor upgrades:
  http://gitorious.org/~jsd/fg/sport-model/commits/sport

I tried asking for suggestions and/or review off-list, 
but it appears that mail to timoor...@gmail.com never 
goes through.  Is it a list-mail address only?



*) Note: Also, I have dreams of implementing a non-blocking
version of this some day ... something that would have been
a nightmare given the readline-style interface.

--
Download Intel® Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
___
Flightgear-devel mailing list
Flightgear-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/flightgear-devel

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

Thanks John,
I'll check this out.
Tim

On Wed, Feb 17, 2010 at 4:09 PM, John Denker  wrote:

> On Mon, Feb 15, 2010 at 5:43 PM, I wrote:
>
> >> 2) It would be even less of a problem to do the following
> >> the specified number of times:
> >>  -- detect the EoF
> >>  -- close the file
> >>  -- reopen the file and start reading again.
> >>
> >> This has the advantage that it works the same as lseek
> >> for regular disk files, and works a whole lot better
> >> for FIFO files.
> >>
> >> It also removes the need for the "sleep" and simplifies
> >> the scenario in item (1) above.
>
> On 02/15/2010 10:36 AM, Tim Moore wrote:
>
> > That would work too.
>
> This is done.
>
> The patches for this can be found at
>  
> http://gitorious.org/~jsd/fg/sport-simgear/commits/sport
>
> Note that libsgio.a now uses boost to provide thread-safe
> user-friendly error messages.  This requires a couple of
> changes to Makefile.am files over on the FG side.  The
> patches for this are available separately:
>  
> http://gitorious.org/~jsd/fg/sport-model/commits/sport
>
> ===
>
> Bottom line:  I believe this fixes all the long-standing
> bugs in SGFile::readline ... and implements the entire
> feature set, including repeat.
>
>  I have not checked the corresponding code in other files
>  such as sg_socket.cxx.  I would not be surprised to learn
>  that there exist other bugs similar to the longstanding
>  sg_file.cxx bugs.
>
>
> --
> 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
>
--
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

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On Mon, Feb 15, 2010 at 5:43 PM, I wrote:

>> 2) It would be even less of a problem to do the following
>> the specified number of times:
>>  -- detect the EoF
>>  -- close the file
>>  -- reopen the file and start reading again.
>>
>> This has the advantage that it works the same as lseek
>> for regular disk files, and works a whole lot better
>> for FIFO files.
>>
>> It also removes the need for the "sleep" and simplifies
>> the scenario in item (1) above.

On 02/15/2010 10:36 AM, Tim Moore wrote:

> That would work too.

This is done.

The patches for this can be found at
  http://gitorious.org/~jsd/fg/sport-simgear/commits/sport

Note that libsgio.a now uses boost to provide thread-safe
user-friendly error messages.  This requires a couple of
changes to Makefile.am files over on the FG side.  The
patches for this are available separately:
  http://gitorious.org/~jsd/fg/sport-model/commits/sport

===

Bottom line:  I believe this fixes all the long-standing
bugs in SGFile::readline ... and implements the entire 
feature set, including repeat.

  I have not checked the corresponding code in other files
  such as sg_socket.cxx.  I would not be surprised to learn
  that there exist other bugs similar to the longstanding 
  sg_file.cxx bugs.

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

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Mon, Feb 15, 2010 at 5:43 PM, John Denker  wrote:

> On 02/15/2010 09:22 AM, Tim Moore wrote:
>
> >> Hint:  The sleep statement ensures that the reader (fgfs)
> >> will not see an EoF at the point where one cat of bytes
> >> ends and the next begins.
>
> > I'd probably do without the "sleep" and write while true; do cat bytes;
> > done >/tmp/pipe.flog & instead.
>
> 1) That doesn't work at the moment (but see below!).
> The problem is that when the first cat completes,
> the reader (fgfs) gets and EoF, and that's the end
> of the show.
>
> I believe that the way I've written it causes the redirection to apply to
the whole for loop, so the file isn't closed after each "cat" command. My
simple-minded tests confirm this, but perhaps I'm missing something.

> 
>
> > It's good that you've removed the lseek from the usual path through
> > readline.
>
> :-)
>
> > But, as I said, your patch breaks a useful command-line option. It
> > would be no big deal to insert an lseek to the beginning of the file in
> your
> > new readline; if the file descriptor doesn't support lseek, then no harm
> > done.
>
>
> 2) It would be even less of a problem to do the following
> the specified number of times:
>  -- detect the EoF
>  -- close the file
>  -- reopen the file and start reading again.
>
> This has the advantage that it works the same as lseek
> for regular disk files, and works a whole lot better
> for FIFO files.
>
> It also removes the need for the "sleep" and simplifies
> scenario in item (1) above.
>
> That would work too.
--
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

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On 02/15/2010 09:22 AM, Tim Moore wrote:

>> Hint:  The sleep statement ensures that the reader (fgfs)
>> will not see an EoF at the point where one cat of bytes
>> ends and the next begins.

> I'd probably do without the "sleep" and write while true; do cat bytes;
> done >/tmp/pipe.flog & instead.

1) That doesn't work at the moment (but see below!).
The problem is that when the first cat completes,
the reader (fgfs) gets and EoF, and that's the end 
of the show.



> It's good that you've removed the lseek from the usual path through
> readline. 

:-)

> But, as I said, your patch breaks a useful command-line option. It
> would be no big deal to insert an lseek to the beginning of the file in your
> new readline; if the file descriptor doesn't support lseek, then no harm
> done.


2) It would be even less of a problem to do the following
the specified number of times:
  -- detect the EoF
  -- close the file
  -- reopen the file and start reading again.

This has the advantage that it works the same as lseek
for regular disk files, and works a whole lot better
for FIFO files.

It also removes the need for the "sleep" and simplifies
scenario in item (1) above.

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

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Mon, Feb 15, 2010 at 2:41 PM, John Denker  wrote:

> On 02/15/2010 03:19 AM, Tim Moore wrote:
>
> > 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.
>
> Please try this story:
>
>   mkfifo /tmp/pipe.flog
>   sleep 100 > /tmp/pipe.flog &
>   while true ; do cat bytes > /tmp/pipe.flog ; done &
>   fgfs --generic=file,in,$rate,/tmp/pipe.flog,$protocol
>
> One good hack deserves another. How do you do that on Windows?

> Now that readline does not do seeks, it can read named
> pipes (FIFOs) just fine, and this opens up all sorts of
> non-hackish way of solving the SIGGRAPH problem ... and
> a host of previously-unsolved problems as well.  For
> example, it allows you to switch from one stream of
> bytes to another without restarting fgfs.
>
> Seems like a win/win to me.
>

> If this story is not good enough, please clarify the
> question.
>
> It's good that you've removed the lseek from the usual path through
readline. But, as I said, your patch breaks a useful command-line option. It
would be no big deal to insert an lseek to the beginning of the file in your
new readline; if the file descriptor doesn't support lseek, then no harm
done. Or you could do something different in FGGeneric. In any case I'm not
going to check in this in until that problem is addressed. If don't feel
like doing that, perhaps I'll do it myself.

> 
>
> Hint:  The sleep statement ensures that the reader (fgfs)
> will not see an EoF at the point where one cat of bytes
> ends and the next begins.
>
> I'd probably do without the "sleep" and write while true; do cat bytes;
done >/tmp/pipe.flog & instead.

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

Re: [Flightgear-devel] SGFile::readline

[John Denker]

On 02/15/2010 03:19 AM, Tim Moore wrote:

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

Please try this story:

   mkfifo /tmp/pipe.flog
   sleep 100 > /tmp/pipe.flog &
   while true ; do cat bytes > /tmp/pipe.flog ; done & 
   fgfs --generic=file,in,$rate,/tmp/pipe.flog,$protocol

Now that readline does not do seeks, it can read named
pipes (FIFOs) just fine, and this opens up all sorts of
non-hackish way of solving the SIGGRAPH problem ... and
a host of previously-unsolved problems as well.  For
example, it allows you to switch from one stream of
bytes to another without restarting fgfs.

Seems like a win/win to me.

If this story is not good enough, please clarify the
question.



Hint:  The sleep statement ensures that the reader (fgfs)
will not see an EoF at the point where one cat of bytes
ends and the next begins.

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

Re: [Flightgear-devel] SGFile::readline

[Tim Moore]

On Sun, Feb 14, 2010 at 10:47 PM, John Denker  wrote:

> The following commit message should be self-explanatory:
>
> commit 224ce694fa8ba7dede0e413b81e5dd52e5e65f15
> Author: John Denker 
> 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