subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bert Huijben" <b...@qqmail.nl>
Subject RE: svn commit: r1501371 - /subversion/trunk/subversion/libsvn_ra_serf/util_error.c
Date Tue, 09 Jul 2013 20:32:00 GMT


> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s@daniel.shahaf.name]
> Sent: dinsdag 9 juli 2013 22:16
> To: Bert Huijben
> Cc: dev@subversion.apache.org; commits@subversion.apache.org
> Subject: Re: svn commit: r1501371 -
> /subversion/trunk/subversion/libsvn_ra_serf/util_error.c
> 
> Bert Huijben wrote on Tue, Jul 09, 2013 at 20:11:57 +0200:
> > Same as for all previous patches in line:
> >
> 
> "All previous patches"?  There was one previous patch along these lines.
> 
> > How is this going to help?
> 
> I already told you how: it is going to help because API users can't do
> anything with the value 120171 that they presently receive.  The
> outermost error must be defined by APR or by Subversion.  120171 is
> neither.

*Why* ?

There is no rule that apr_err must be set to something that is defined by
APR or Subversion.



Besides APR *defines* everything in a range to be user defined..


This same reasoning applies to wrapping everything to APR_EGENERAL, which is
just as bad.

Every integer value is valid here... And allows checking for it.

APR_EGENERAL (=1) doesn't allow checking for by api users. And bindings
can't translate textual error messages back to text.


So *why* do *you* want to change the existing error code to this wrapping
code that is only used wrapping for non-bucket errors.  (Which is a specific
set of APR defined error codes that are handled specificly in serf).

I would reason like Raymond Chen (of the oldnewthing fame) here: every
change starts at -1000 points. And needs a good reason to be implemented as
new feature, and an even better reason to be backported as 'fix' to a
maintenance branch.


	Bert


Mime
View raw message