Stefan Sperling wrote on Mon, Feb 03, 2014 at 21:02:50 +0100: > Thoughts? > > @@ -316,6 +317,9 @@ svn_cl__blame(apr_getopt_t *os, > "mode")); > } > > + if (!opt_state->non_interactive && !opt_state->xml) > + SVN_ERR(svn_cl__start_pager(&pager_proc, pool, pool)); > + > for (i = 0; i < targets->nelts; i++) > { > svn_error_t *err; > @@ -409,6 +413,9 @@ svn_cl__blame(apr_getopt_t *os, > if (opt_state->xml && ! opt_state->incremental) > SVN_ERR(svn_cl__xml_print_footer("blame", pool)); > > + if (pager_proc) > + SVN_ERR(svn_cl__close_pager(pager_proc, pool)); > +
Do you need to call __close if there is an error after __start? i.e., is there a state in which stdout will be printed to in the error path? What about the case SVN_PAGER="/nonexistent"? Should it fall back to just printing to stdout, or bail out on account of failing to execute "/nonexistent"?