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

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

 



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@xxxxxxxxxxxx  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]