Re: [Xen-devel] [PATCH]: xl: don't segfault parsing disk configs, support NULL physpath and ioemu:

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

On Thu, 2010-08-19 at 15:54 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("[Xen-devel] [PATCH]: xl: don't segfault parsing disk 
> configs, support NULL physpath and ioemu:"):
> > Switch to a state machine parser since it's easier to handle all these
> > exotic cases without segfaulting. NULL physpaths are now allowed and a
> > dodgy hack is introduced to skip over the "ioemu:" prefix for a
> > virtpath. Also fixes a leak of buf2.
> 
> I'm not convinced that it is clearer.  It's certainly a lot longer:
> 132 lines added for 50 removed.

It's true that it's longer but the nature of these types of parsers it's
a lot of very short lines :)

I think it's clearer than a correct strtok() + handling all errors and
variations would be.

> Perhaps we should just import a regexp parser and use that ?  Really,
> we should have a regexp or some other kind of declarative statement of
> the syntax.
> 
> Possible regexp parsers include pcre and flex.  flex has the advantage
> that we're using it already so it's just another line in the
> Makefile.

It's your call, I know nothing of flex and its mysterious ways and my
pcre skills are limited to basic text-editor-fu... I agree that flex
probably makes the most sense.

On the other hand whoever designed these formats seemed to want to make
them difficult to parse. Since it's all python I find myself wondering
why they didn't use a dictionary or a tuple.

Gianni


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