[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] (absolutePathFromBaseFile): fix up preceding commit



On Wed, Feb 10, 2010 at 12:30:26PM +0100, Jim Meyering wrote:
> Daniel P. Berrange wrote:
> >> -    virAsprintf(&res, "%.*s/%s", base_file, d_len, path);
> >> +    /* Ensure that the following cast-to-int is valid.  */
> >> +    assert (d_len <= INT_MAX);
> >> +
> >> +    ignore_value(virAsprintf(&res, "%.*s/%s", (int) d_len, base_file, path));
> >>      return res;
> >>  }
> >
> > NACK to this
> 
[...]
> I'm not sure how a user or even a malicious admin could cause base_file
> to have a length of 2GiB and pass it to this function, but it's easy
> to avoid the assertion this time, so I've done so.

  It's a bit of an unwritten policy, but really we should try to avoid
assert() in libvirt code, even if in that case right it's look unlikely
to get there, but it's better to handle things gracefully, as long as
it doesn't make the code really really worse. Point is if we start to
add assert() calls, then new people looking at the code and submitting
new patches may thing it's generally acceptable, so let's avoid this
as much as possible,

  thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]