You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2004/05/16 19:46:11 UTC

svn_cmdline_printf and errors

Hi,

As the new functions svn_cmdline_printf, svn_cmdline_fprintf and
svn_cmdline_fputs are defined, they return an svn_error_t *, but the
implementation never fails. This is because it uses the fuzzy conversion
from UTF-8 if the real one fails, and it doesn't check the return value of
the underlying fputs call.

IN the current code, non of the printf/fprintf/fputs calls check for
errors. Ofcourse this is because if they all output to stdout or stderr
and if that doesn't work, there is not much to do. You can't inform the
user anyway.

We could either:
a) Use SVN_ERR everywhere we call one of the new functions except in
svn_handle_error. Is this overkill?
b) Ditch the return value of the new functions in favour of void and
ignore failures (hoping for SIGPIPE kicking in as we do today).

Opinions?

Regares,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Branko Čibej <br...@xbc.nu>.
Peter N. Lundblad wrote:

>>This is for printing progress and error messages to the command line. I
>>think we should act like normal printf, FWIW, apr_vformatter returns the
>>character count.
>>
>>    
>>
>You mean returning an int which can be negative in case of failure?
>  
>
apr_vformatter can't error out; it can only crash on out-of-memory

>Do we agree that the current return value of svn_error_t* is wrong?
>  
>
I think so.




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Erik Huelsmann <e....@gmx.net>.
> On Sun, 2004-05-16 at 17:31, Peter N. Lundblad wrote:
> > Why do we ignore SIGPIPE?
> 
> A Unix program which uses the network must ignore SIGPIPE or it will be
> unreliable in the face of lost connections.
> 
> Even disregarding that, if you run "svn log file://blah | less" and quit
> out of less, we need to close the database or we could leave it wedged.
> 
> We should not be ignoring printf errors, but SIGPIPE is not the answer.

Which implies that it is ok to return svn_error_t *'s?

BTW: You all know there is an issue on the subject?

http://subversion.tigris.org/issues/show_bug.cgi?id=780


bye,


Erik.

-- 
"Sie haben neue Mails!" - Die GMX Toolbar informiert Sie beim Surfen!
Jetzt aktivieren unter http://www.gmx.net/info


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:

>On Sun, 2004-05-16 at 18:12, Branko Čibej wrote:
>  
>
>>Oh well, if whoever replaces all the printfs/fputs's with these 
>>functions will add the SVN_ERR's, then I'll agree to this. Sometimes 
>>it's nice to know how many chars you wrote, and we'll be forever hiding 
>>that info; but since we're not using it now...
>>    
>>
>
>Can't you find that out with %n?
>
>  
>
Come to think of it, you can.

/me needs a cluebat today



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-05-16 at 18:12, Branko Čibej wrote:
> Oh well, if whoever replaces all the printfs/fputs's with these 
> functions will add the SVN_ERR's, then I'll agree to this. Sometimes 
> it's nice to know how many chars you wrote, and we'll be forever hiding 
> that info; but since we're not using it now...

Can't you find that out with %n?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn_cmdline_printf and errors

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 16 May 2004, [UTF-8] Branko Ä^Libej wrote:

> Greg Hudson wrote:
>
> >On Sun, 2004-05-16 at 17:58, Peter N. Lundblad wrote:
> >
> >
> >>Hmmm... Isn't SVN_ERR (svn_cmdline_printf (...)); the only answer then?
> >>:-)
> >>
> >>
> >
> >I'm fine with that.  I don't know why Branko thinks we should return
> >similar values to what printf returns.  It's not like
> >svn_io_file_close() tries to be consistent with close(), and so forth.
> >
> >
> Oh well, if whoever replaces all the printfs/fputs's with these
> functions will add the SVN_ERR's, then I'll agree to this. Sometimes

I was planning to start that this evening, if not for that return value...
It's boring anyway, so I'll do that:-)

> it's nice to know how many chars you wrote, and we'll be forever hiding
> that info; but since we're not using it now...

If you need that, you can always use sprintf (oh, no, apr_psprintf:-)
first. But still you get the number of *bytes*, not the numer of
characters. Is that really that useful?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn_cmdline_printf and errors

Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:

>On Sun, 2004-05-16 at 17:58, Peter N. Lundblad wrote:
>  
>
>>Hmmm... Isn't SVN_ERR (svn_cmdline_printf (...)); the only answer then?
>>:-)
>>    
>>
>
>I'm fine with that.  I don't know why Branko thinks we should return
>similar values to what printf returns.  It's not like
>svn_io_file_close() tries to be consistent with close(), and so forth.
>  
>
Oh well, if whoever replaces all the printfs/fputs's with these 
functions will add the SVN_ERR's, then I'll agree to this. Sometimes 
it's nice to know how many chars you wrote, and we'll be forever hiding 
that info; but since we're not using it now...




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-05-16 at 17:58, Peter N. Lundblad wrote:
> Hmmm... Isn't SVN_ERR (svn_cmdline_printf (...)); the only answer then?
> :-)

I'm fine with that.  I don't know why Branko thinks we should return
similar values to what printf returns.  It's not like
svn_io_file_close() tries to be consistent with close(), and so forth.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 16 May 2004, Greg Hudson wrote:

> On Sun, 2004-05-16 at 17:31, Peter N. Lundblad wrote:
> > Why do we ignore SIGPIPE?
>
> A Unix program which uses the network must ignore SIGPIPE or it will be
> unreliable in the face of lost connections.
>
I see.

> We should not be ignoring printf errors, but SIGPIPE is not the answer.
>
Hmmm... Isn't SVN_ERR (svn_cmdline_printf (...)); the only answer then?
:-)

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sun, 2004-05-16 at 17:31, Peter N. Lundblad wrote:
> Why do we ignore SIGPIPE?

A Unix program which uses the network must ignore SIGPIPE or it will be
unreliable in the face of lost connections.

Even disregarding that, if you run "svn log file://blah | less" and quit
out of less, we need to close the database or we could leave it wedged.

We should not be ignoring printf errors, but SIGPIPE is not the answer.


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Sun, 16 May 2004, [UTF-8] Branko �^Libej wrote:

> Tobias Ringström wrote:
>
> > Peter N. Lundblad wrote:
> >
> >> b) Ditch the return value of the new functions in favour of void and
> >> ignore failures (hoping for SIGPIPE kicking in as we do today).
> >>
> >> Opinions?
> >
> >
> > FWIW, we currently set the SIGPIPE handler to SIG_IGN, which means
> > that we depend on getting an error return value from the write functions.
>
Why do we ignore SIGPIPE? If we do that and at the same time ignore printf
errors, we won't notice a broken pipe. So if I do
svn ls -R |less
and then quit less after one page, won't svn keep retrieving the whole
tree then?

> This is for printing progress and error messages to the command line. I
> think we should act like normal printf, FWIW, apr_vformatter returns the
> character count.
>
You mean returning an int which can be negative in case of failure?

Do we agree that the current return value of svn_error_t* is wrong?

//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn_cmdline_printf and errors

Posted by Branko Čibej <br...@xbc.nu>.
Tobias Ringström wrote:

> Peter N. Lundblad wrote:
>
>> b) Ditch the return value of the new functions in favour of void and
>> ignore failures (hoping for SIGPIPE kicking in as we do today).
>>
>> Opinions?
>
>
> FWIW, we currently set the SIGPIPE handler to SIG_IGN, which means 
> that we depend on getting an error return value from the write functions.

This is for printing progress and error messages to the command line. I 
think we should act like normal printf, FWIW, apr_vformatter returns the 
character count.

-- Brane



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
Peter N. Lundblad wrote:
> b) Ditch the return value of the new functions in favour of void and
> ignore failures (hoping for SIGPIPE kicking in as we do today).
> 
> Opinions?

FWIW, we currently set the SIGPIPE handler to SIG_IGN, which means that 
we depend on getting an error return value from the write functions.

/Tobias

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by Branko Čibej <br...@xbc.nu>.
Peter N. Lundblad wrote:

>On Sun, 16 May 2004, [UTF-8] Branko Ä^Libej wrote:
>
>  
>
>>    $ man printf
>>    ...
>>    int printf (const char *fmt, ...)
>>    ...
>>    returns the number of characters written...
>>
>>
>>I don't see any reason change that.
>>
>>    
>>
>According to the man page for fputs, it returns the number of bytes
>written or EOF on error and according to the glibc manual, printf returns
>a negative numer on error.
>
>So you don't see a reason to change what?
>  
>
I'd keep these functions as close to standard as makes sense. Here's 
what the standard has to say:

    7.19.6.1p14: The fprintf function returns the number of characters
    transmitted, or a negative value if an output or encoding error
    occurred.

    7.19.6.3p2: The printf function is equivalent to fprintf with the
    argument stdout interposed before the arguments to printf.

    7.19.6.3p3: The printf function returns the number of characters
    transmitted, or a negative value if an output or encoding error
    occurred.

    7.19.7.4p3: The fputs function returns EOF if a write error occurs;
    otherwise it returns a nonnegative value.

    7.19.1p3: EOF which expands to an integer constant expression, with
    type int and a negative value, ...

So note that according to the standard, all three of these functions can 
return EOF on error and the number of transmitted chars otherwise. Which 
is what I propose they do. That will also satisfy the requirements for 
the streaming output functions (if they call any of these), because they 
only need to know that an output error occurred, not _which_ error 
occurred. I'd hate to see us have to add SVN_ERR everywhere in the code.

-- Brane



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn_cmdline_printf and errors

Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.

On Sun, 16 May 2004, [UTF-8] Branko Ä^Libej wrote:

>     $ man printf
>     ...
>     int printf (const char *fmt, ...)
>     ...
>     returns the number of characters written...
>
>
> I don't see any reason change that.
>
According to the man page for fputs, it returns the number of bytes
written or EOF on error and according to the glibc manual, printf returns
a negative numer on error.

So you don't see a reason to change what?

Regards,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: svn_cmdline_printf and errors

Posted by Branko Čibej <br...@xbc.nu>.
   $ man printf
    ...
    int printf (const char *fmt, ...)
    ...
    returns the number of characters written...


I don't see any reason change that.

Peter N. Lundblad wrote:

>Hi,
>
>As the new functions svn_cmdline_printf, svn_cmdline_fprintf and
>svn_cmdline_fputs are defined, they return an svn_error_t *, but the
>implementation never fails. This is because it uses the fuzzy conversion
>from UTF-8 if the real one fails, and it doesn't check the return value of
>the underlying fputs call.
>
>IN the current code, non of the printf/fprintf/fputs calls check for
>errors. Ofcourse this is because if they all output to stdout or stderr
>and if that doesn't work, there is not much to do. You can't inform the
>user anyway.
>
>We could either:
>a) Use SVN_ERR everywhere we call one of the new functions except in
>svn_handle_error. Is this overkill?
>b) Ditch the return value of the new functions in favour of void and
>ignore failures (hoping for SIGPIPE kicking in as we do today).
>
>Opinions?
>
>Regares,
>//Peter
>  
>




---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org