On Thu, 2011-03-31 at 18:30 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("Re: [Xen-devel] [PATCH] libxl: new xlu_disk_parse
> function"):
> > On Thu, 2011-03-24 at 17:49 +0000, Ian Jackson wrote:
> > > + return;
> >
> > case 1: ??
>
> Deliberately omitted.
>
> > > + case 2:
> > > + case 3:
> > > + case 4:
> > > + break;
> >
> > Or does it belong here? In which case aborting on a parse error is bad
> > juju.
> > case 1:
> > > + default:
> > > + abort();
>
> I could add it there for clarity. The regexp will always match
> capturing with 2, 3 or 4 parens. None of the other errors from
> dfa_exec are applicable. So anything other than 2,3,4 or "did not
> match" is due to a bug in the code, not merely bogus input. Perhaps
> this should be mentioned in a comment.
I think that's a good idea, you'll only up nacking an endless stream of
fixup patches otherwise (inevitably adding the case in the wrong
place...).
IMHO a switch statement in this context obfuscates the error handling
and a couple of error handling if statements would be more obvious e.g.
if (rc == PCRE_ERROR_NOMATCH) {
xlu__disk_err(dpc, "bad syntax for target, or missing vdev");
return;
}
/* The regexp will always match capturing with 2, 3 or 4 parens */
if (rc < 2 || rc > 4)
abort();
.. carry on...
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home