Re: [PATCH 8/8] storage: Guess the parent if it's not specified for vHBA

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

 



On 02/06/2013 11:32 AM, John Ferlan wrote:

>>>> +
>>>> +        ignore_value(sscanf(entry->d_name, "host%d",&host));
>>>
>>> Why ignore_value()?  If == -1, then host is undefined - could be
>>> something that results in the following being successful..
>>
>> I don't think it's possible to return -1, as all the entries
>> under SYSFS_FC_HOST_PATH should be "hostN". Entry "." and ".."
>> are already skipped.
>>
> 
> sscanf can return -1 for other errors (ENOMEM included) - I would think
> it's safer to check and fail than assume anything.

sscanf should NEVER be used to parse raw "%d".  POSIX says that the
result is undefined on integer overflow (sscanf is not required to fail
in that case, yet you are not guaranteed if you got MAX_INT, wraparound
modulo 2**32, or some other weird behavior).  Something like
sscanf("%5d") for parsing at most five digits is slightly more tolerable
because it prevents you from getting to the overflow situation, although
I still think that using sscanf to parse integers is dangerous.  And
even in the cases where using sscanf is safe (fixed-length parsing not
subject to integer overflow), you should NEVER ignore failure, and you
should always end with a %n to ensure that you parsed as much as you
were expecting to parse.  sscanf() is so difficult to correctly use
directly that I recommend that we avoid adding any new uses of it into
libvirt (I'd like to turn on the 'make syntax-check' rule that forbids
*scanf entirely, but we'd have to first convert our existing uses into
alternative code).

I'd much rather see virStrToLong_i() used here.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]