Peter N. Lundblad wrote:
> On Mon, 30 May 2005, Julian Foad wrote:
>>>+ case svn_node_file:
>>>+ str_kind = "file";
>>>+ break;
>>>+
>>>+ case svn_node_dir:
>>>+ str_kind = "directory";
>>>+ break;
>>>+
>>>+ case svn_node_none:
>>>+ str_kind = "none";
>>>+ break;
>>>+
>>>+ case svn_node_unknown:
>>>+ default:
>>>+ str_kind = "unknown";
>>
>>The "default" case should throw an error, I should think. (I see you just
>>copied this from the existing non-XML code.)
>
> I'm not sure about this. Regarding the status in XML patch, I suggested
> using abort(). My reasoning is that if we have a switch that covers all
> enum values, getting another value is an API violation. We have lots of
> cases where we just crash when something violates the API (we even have
> some abort() cases). For example, we normally don't check for a NULL
> pointer, unless that's explicitly allowed. Adding error messages for such
> cases that "cannot happen" just clutters the code IMO. And, as a minor
> thing, it makes the translators have to translate really obscure errors.
I agree. By "throw an error" I meant "return an error object, or abort, or
whatever".
Later in the thread I commented that "abort" might not be liked. I got this
impression from a few months ago when I recommended using "assert" more than we
currently do, and it seemed that the shouts of objection seemed were louder
than the voices of agreement. Nothing was really decided, and I dropped the
discussion, hoping to revisit it later.
We need a project policy on how to handle this type of error, as it is a
difficult topic, so maybe now is the time to have that discussion.
For this particular case (node kind is anything but file or directory, in fact)
I agree, especially having seen the same in svnserve.c, that "abort()" is the
best action (unless, of course, there is any legitimate way in which "none" or
"unknown" can be obtained).
> OTOH, I can understand arguments for returning an error instead. IN that
> case, maybe we should have an API violation error which always includes
> file and line info, or alternatively has a message attatched that doesn't
> need to be translated.
Maybe.
> Note that I think abort is the right thing when some API returns an
> illegal value, not when the WC got corrupt or something like that, but
> that's obvious, isn't it?
Yes, of course.
- Julian
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 1 00:44:39 2005