[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1501371 - /subversion/trunk/subversion/libsvn_ra_serf/util_error.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Tue, 9 Jul 2013 22:32:00 +0200

> -----Original Message-----
> From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
> Sent: dinsdag 9 juli 2013 22:16
> To: Bert Huijben
> Cc: dev_at_subversion.apache.org; commits_at_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.

Received on 2013-07-09 22:33:02 CEST

This is an archived mail posted to the Subversion Dev mailing list.