Re: [PATCH for-4.6 1/2] libxl: fix devd removal path

[Ian Campbell <ian.campbell@citrix.com>]

On Wed, 2015-09-23 at 11:36 +0200, Roger Pau Monné wrote:
> El 23/09/15 a les 10.46, Ian Campbell ha escrit:
> > On Tue, 2015-09-22 at 18:23 +0200, Roger Pau Monne wrote:
> > >  
> > > -    /* Check if event_path ends with "state" and truncate it */
> > > -    if (strlen(event_path) < strlen("state"))
> > > +    /* Check if event_path ends with "state" or "online" and
> > > truncate it. */
> > > +    if (strlen(event_path) < strlen("state") ||
> > > +        strlen(event_path) < strlen("online"))
> > 
> > This is the same as strlen(...) < strlen("state") (or more formaly <
> > min(strlen("state"),strlen("online")).
> > 
> > Which is a bit dangerous because one might be tricked into thinking
> > that
> > path - strlen("online") was safe to do after this check when it's not.
> > (The
> > new code seems to have avoided this particular pattern which the old
> > code
> > used though)
> 
> With the new code this is no longer needed, so I'm tempted to just
> remove it.
> 
> > I think I agree with Ian's suggestion to use strrchr.
> > 
> > Alternatively you could register two watches on the two specific paths
> > you
> > care about and point them at wrappers which trivially strip the
> > appropriate
> > trailing text and pass the base onto the current code?
> > 
> > BTW, what is watch_path passed to this function, is it not the watched
> > path
> > i.e. the exact truncated event_path that you are looking for?
> 
> Watch path is /local/domain/<domid>/backend so we can get an event for
> any backend that is added to the domain. We could add specific watches
> once we have identified backends that the driver domain needs to serve,
> but it's going to make the code more complex and at this point in the
> release I don't think that's a good idea.

Ah, my mistake I thought we were already watching the actual backend dirs
one by one. Since we aren't I don't think it makes sense to change, and
having a single global watch is preferable anyway.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel