WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] libxl: new xlu_disk_parse function

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.

> Hmm, I'm not sure this is nicer than the code it's replacing, it's
> certainly a lot longer. I don't like the idea of it being full of things
> comparing for ",w" or ":cdrom" etc, rather than tokenising it fully and
> evaluating what the tokens are separately.

Perhaps you're right.  Unfortunately the nasty multi-level nature of
this parsing problem makes this a bit awkward.

But I think I can remove the delimiters into the regexp which perhaps
will help.

> > +raw:           { DSET(format,FORMAT,RAW); }
> > +qcow:          { DSET(format,FORMAT,QCOW); }
> > +qcow2:         { DSET(format,FORMAT,QCOW2); }
> > +vhd:           { DSET(format,FORMAT,QCOW2); }
> > +
> > +phy:           { DSET(format,FORMAT,RAW); DSET(backend,BACKEND,PHY); }
> > +file:          { DSET(format,FORMAT,RAW); DSET(backend,BACKEND,TAP); }
> > +tapdisk:|tap2?:        { DSET(backend,BACKEND,TAP); }
> > +aio:           { }
> > +ioemu:         { }
> 
> This bit is quite nice though. We could probably just tidy up the
> existing parser using arrays of values and things rather than a lot of
> if/else statements though.

I wanted to avoid parsing with pointer arithmetic, which is very easy
to write bugs in - particularly when new features are added.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel