Re: CVS update: subversion/subversion/client diff.c

[Ben Collins-Sussman <su...@newton.collab.net>]

OK, so why did I have to make this change?  I mean, we went through
all this bother to convert stdout into an apr_file_t, but I'm not
supposed to apr_close() it when I'm done?

I noticed this weirdness when I specified multiple targets to diff;
only the first diff would display -- because apparently the
"apr_close()" of stdout would somehow shut off the stream for all the
remaining diff calls.

I'm just wondering if this is a bug or a feature.  :)


sussman@tigris.org writes:

>   User: sussman 
>   Date: 01/01/26 11:40:49
> 
>   Modified:    subversion/client diff.c
>   Log:
>   (svn_cl__print_file_diff):  stop apr_closing the handle which
>   represents stdout.  This was preventing us from printing out diffs for
>   more than one file on the commandline.
>   
>   Revision  Changes    Path
>   1.5       +0 -2      subversion/subversion/client/diff.c
>   
>   Index: diff.c
>   ===================================================================
>   RCS file: /cvs/subversion/subversion/client/diff.c,v
>   retrieving revision 1.4
>   retrieving revision 1.5
>   diff -u -r1.4 -r1.5
>   --- diff.c	2001/01/26 19:15:18	1.4
>   +++ diff.c	2001/01/26 19:40:48	1.5
>   @@ -69,8 +69,6 @@
>                                         NULL, outhandle, NULL, pool);
>      if (err) return err;
>      
>   -  apr_close (outhandle);
>   -
>      /* TODO:  someday we'll need to worry about two things here:
>    
>         1.  svn_client_file_diff may be returning a file from RA instead
>   
>   
>

Re: CVS update: subversion/subversion/client diff.c

[RADICS Peter <mi...@lbcons.net>]

On Fri, Jan 26, 2001 at 01:49:43PM -0600, Ben Collins-Sussman wrote:
[SNIP]
> I noticed this weirdness when I specified multiple targets to diff;
> only the first diff would display -- because apparently the
> "apr_close()" of stdout would somehow shut off the stream for all the
> remaining diff calls.
>
> I'm just wondering if this is a bug or a feature.  :)

Well, if you close stdout, then it's *err* closed :)
what did you expect really?  stdout is just a normal stream, with a
special name.  So I won't call it a bug.
Feel free to flame me if I'm completely wrong here though. :)

cheers,
mitch
-- 
// RADICS Peter <mi...@lbcons.net> (http://lbcons.net)
//
// "If human beings don't keep exercising their lips, 
//  their brains start working." -- Ford Prefect

closing stdout (was: Re: CVS update: subversion/subversion/client diff.c)

[Greg Stein <gs...@lyra.org>]

On Sat, Jan 27, 2001 at 11:48:58AM -0500, Greg Hudson wrote:
> > OK, so why did I have to make this change?  I mean, we went through
> > all this bother to convert stdout into an apr_file_t, but I'm not
> > supposed to apr_close() it when I'm done?
> 
> I'm certainly not surprised; when you fdopen() a file descriptor, the
> resulting file handle "owns" the file descriptor and will close it
> when you fclose() the file handle.  Same with APR and
> apr_put_os_file() or whatever it's called.
> 
> This would be cleaner if apr_create_process had a better interface for
> "leave stdout alone."  (Assuming it doesn't... if I recall correctly,
> you said that if outfile was NULL then stdout went nowhere.)

I think the right behaviour is to dup() the stdout descriptor, thus allowing
the file to be closed without problem.

Note that a pool cleanup closes a file. If you had done the allocation in a
subpool and then cleared it... bam! There goes the file.

Need to investigate some more, but I'll make a note of it in APR's STATUS
file.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/