* Zefram <zef...@fysh.org> [2015-07-19 13:05]:
> James E Keenan wrote:
> >                     That leaves only the problem of capturing STDOUT
>
>     close(STDOUT);
>     open(STDOUT, ">", \$captured_stdout);

The code never explicitly prints to a particular handle and doesn’t call
out, so playing the dup/reopen game seems unnecessary to me… Would you
argue that the tests require that level of defensiveness (which is still
imperfect, mind)?


* James E Keenan <jk...@verizon.net> [2015-07-19 14:25]:
> Thanks for the suggestions. I had looked at 'perldoc -f select' but
> couldn't quite figure out the sequence of commands.

What about

    $oldfh = select(STDERR); $| = 1; select($oldfh);

is unclear?

> This got me much closer.

It didn’t. What you ended up making of the pieces your were given is
broken. You would have had something that works (with some asterisks) if
you had merely copied my amendment of your example code verbatim; or you
had to understand Zefram’s abridged pointer to another solution and
implement it properly. But pasting together garbled incomplete pieces of
the suggestions does not meet either requirement.

> Here's what I currently have, which is DWIMming:
>
> sub _run_for_verbose {
>      my $coderef = shift;
>      my $stdout;
>      my $oldfh = select(STDOUT);
>      local $| = 1;
>      close STDOUT;
>      open STDOUT, '>', \$stdout;
>      &$coderef;
>      close STDOUT;
>      return $stdout;
> }
>
> Called like:
>
>      is(_run_for_verbose(sub {@created = mkpath($dir, 1)}),
>          "mkdir $base\nmkdir $dir\n",
>          'mkpath verbose (old style 1)'
>      );
>
> Thank you very much.

So relative to your example code in the first mail, you added the
following lines:

    my $oldfh = select(STDOUT);
    local $| = 1;
    close STDOUT;
    open STDOUT, '>', \$stdout;
    close STDOUT;

Out of all these lines, only the last two actually have an effect. The
others do nothing. To say that this got you “much closer” means you are
engaged in textbook programming by coincidence.

https://pragprog.com/the-pragmatic-programmer/extracts/coincidence

File::Path is a very visible, high-upriver module with serious security
sensitivity. It has been the subject of CVEs before.

If you lack the expertise to maintain it then please do not attempt to.
Unless you are willing to be responsible for security bulletins about
Perl.

Note that I imply no claim to possess that expertise myself (as should
become abundantly clear below). I also do not at all want to discourage
you from learning – quite the opposite. Not even from learning by trial
and error (though I hope you do consider that article). I am well aware
that expertise comes from experience, which has to be gained.

However, please do not make a piece of code as critical and sensitive as
this into your personal learning experience.


* Zefram <zef...@fysh.org> [2015-07-19 15:00]:
> In case the &$coderef produces no output, you should initialise
> $stdout to the empty string, not leave it undef.

Good catch.

> You should consider the behaviour on exceptions. Whatever state
> restoration you want to perform should be somehow tied to the stack
> frame, so that it happens automatically on unwinding, rather than
> relying on your function remaining in control.

Hm, even better catch. There is no way I can think of doing this with
the select-based approach, nor with dup/reopen without involving XS;
however, localising the Perl-level filehandle rather than doing an
OS-level dup/reopen seems to work fine, i.e.:

  sub _run_for_verbose {
     my $coderef = shift;
     my $out = '';
     local *STDOUT;
     open STDOUT, '>', \$out;
     &$coderef;
     return $out;
  }

This might or might be deemed sufficiently defensive. It would not be
enough for code that spawns other processes, obviously. Aside from that
though, I just cooked this up in a moment of superficial dabbling; are
there other glaring problems with this I’m missing?

Regards,
-- 
Aristotle Pagaltzis // <http://plasmasturm.org/>

Reply via email to