I wrote: > I'm not quite sure whether the reached_eof test should be > "if (bytesread == 0)" or "if (bytesread < maxread)". The former > seems more certain to indicate real EOF; are there other cases where > the fread might return a short read? On the other hand, if we > support in-band EOF indicators (\. for instance) then we might > stop without having made an extra call to CopyGetData that would > see bytesread == 0. On the third hand, if we do do that it's > not quite clear to me whether SIGPIPE is ignorable or not.
After still further thought, it seems like "if (bytesread == 0)" is the right way to proceed. That protects us against incorrectly setting reached_eof if the pipe is being run in unbuffered or line-buffered mode. (Which copy.c doesn't do, at the moment, but I'd just as soon this code didn't need to assume that. Also, I'm not 100% convinced that Windows or other platforms might not act that way.) In the case where we terminate early because we saw an in-band EOF marker, we would also not set reached_eof, and again that seems like a good outcome: if we see SIGPIPE it must mean that the program kept sending data after the EOF marker, and that seems like a good thing to complain about. (It's only guaranteed to fail if the program sends more than the current pipe buffer-ful, but I don't feel a need to add extra code to check for nonempty buffer contents as we exit.) So I think this version is probably good, although maybe it could use an additional comment explaining the above reasoning. regards, tom lane