Based on my testing, it looks like body_resetpos() behavior would be
unaffected by the dup(). I didn't realize this was the case, so my
mention of 'if we happen to seek() to the wrong place bad things won't
happen' is not exactly accurate. But this is probably for the best;
although I would normally expect someone to do '$txn->body_resetpos; my
$fh = $txn->body_fh;' -- it is probably best to not break when they do
'my $fh = $txn->body_fh; $txn->body_resetpos;'. So the main
accomplishment of calling dup() is that we can pass $fh to a module that
calls $fh->close() without clobbering things :) I'll send in a patch
soonish.
-Jared
Robert Spier wrote:
Most plugins call body_resetpos before using the FH.
-R
Jared Johnson wrote:
I'm considering submitting a patch that enhances
Qspmtpd::Transaction::body_fh() to (1) call body_spool() if the
message was not already spooled to disk; and (2) dup the filehandle
before returning it, so that if a plugin that calls body_fh, or an
external module that the plugin passes the fh to, happens to seek() to
some weird place or close() the fh, bad things won't happen to plugins
that use the fh later (body_fh, body_resetpos, body_getline).
My question is, are there times when a plugin might be expecting such
modifications to body_fh, justifying splitting the second
functionality into a separate function, say, body_fh_dup() or so? I'd
lean toward making the changes directly to body_fh() but wanted to see
if there were any objections to this.
-Jared
