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