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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 31 Mar 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] libxl: new xlu_disk_parse 
> function"):
> > On Mon, 28 Mar 2011, Gianni Tedesco wrote:
> > > 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.
> > 
> > I definitely agree.
> > Besides when doing refactoring the code produced should be either shorter or
> > at least easier to read but this code is neither of them.
> 
> I agree that as a rule of thumb better code is shorter.  Unfortunately
> string parsing in C is one of those exceptions.
> 
> What I was trying to do was to separate out the actual syntax in a way
> that is reasonably tractable, and which is amenable to being edited
> later without introducing bugs.
> 
> So while the current code is longer in this case I think it's clearer.
> A good deal of the added code is very simple and straightforward "set
> up this regexp engine and check the return value".

The problem with this code is that, exactly like regex in general, is
write-only.
In one year time making a change to this code would be harder than
rewrite it again from scratch for the third time.
I vote for the explicit state machine.

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