[Toybox] Outsmarting myself again...

[Rob Landley]

So I wrote all this code and then mothballed it ~3 years ago, and now
going back through it I'm staring at some of it trying to work out what
I was thinking.

Case in point, lib/lib.c has loopfiles(), which calls loopfiles_rw().
This iterates through an array of filenames (generally the command line
arguments left over from option parsing, I.E. toys.optargs) opens each
one (detecting "-" means stdin), and then calls a function pointer on
each with the filehandle and file name.

The reason for loopfiles() vs loopfiles_rw() is that loopfiles() default
behavior is to warn about files that don't exist (and eventually exit
with a nonzero error code), but not to die immediately.  It also doesn't
create the files.  The loopfiles_rw() variant instead takes file open
flags and permissions, so it can create the files named in its arguments
if necessary.

So "cat" uses loopfiles(), but "touch" uses loopfiles_rw().

Problem: when the function pointer returns, if the flags argument was
zero, loopfiles_rw() closes the filehandle.  But not if flags wasn't zero.

*boggle*

Now O_RDONLY is in fact 0 (and that's posix), so it closes the
filehandle for the loopfiles() case but not the loopfiles_rw() case.
Presumably what I was thinking was that when we're creating/writing
files the function needs to flush data and made sure it didn't get an
error, so it needs to perform error handling (potentially even on close,
due to NFS being horrible, although I'm not sure how much I care about
NFS), so it's the called function's job to close the filehandle in the
write case, and the wrapper's job to close it in the read case.

I think.

But dude, that's way too subtle and I didn't even bother to COMMENT it
and I think I need to file off the pointy bit here and pick something
and be CONSISTENT.  Or at least document why I wasn't, because I'm not
remembering it 3 years later...

Oh, hang on, I did leave a comment, at the top of the function:

// Note: read only filehandles are automatically closed when function()
// returns, but writeable filehandles must be close by function()

(And this is _why_ I compulsively document all this stuff...)

Rob

P.S.  The reason this came up is that "cmp" has its own "-" handlign
logic, and I'm trying to figure out if I should involve loopfiles or
just factor out opendash() as a function.  I'm actually leaning towards
abusing loopfiles, since it's already there...

 1328673775.0