2010/3/30 Jim Meyering <jim@xxxxxxxxxxxx>:
> Matthias Bolte wrote:
>> src/conf/nwfilter_conf.c | 10 +++++-----
>
>
> Hi Matthias,
>
> It's great that you're removing all of these sscanf uses.
> I suppose the plan includes eventually enabling the syntax-check
> that prohibits them altogether.
Yes, once scanf and atoi are gone, that rule should be activated.
>> diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
>> @@ -1226,7 +1226,7 @@ virNWFilterRuleDetailsParse(virConnectPtr conn ATTRIBUTE_UNUSED,
>> - if (sscanf(prop, "%d", &int_val) == 1) {
>> + if (virStrToLong_i(prop, NULL, 10, &int_val) >= 0) {
>
> Not sure it's worth worrying about, but bear in mind
> that this patch does induce a semantic change:
> sscanf is more permissive, and returns "1" even if there's
> garbage in the "prop" string after a valid integer, while
> virStrToLong_i (with NULL param #2) will reject that same
> bogus input string.
Yes, I'm aware of that. In some cases it is necessary to pass a
non-NULL value as second parameter to virStrToLong_i in order to have
the same behavior as scanf. For example the new virParseVersionNumber
function does this in order to ignore the suffix on the VirtualBox
version number 3.1.4_OSE.
> I think of this as a feature, but it probably deserves
> a note in the commit log, so if some libvirt client starts
> seeing mysterious new failures due to their previously-accepted
> bogus inputs, they might find this set of commits.
True this needs to be mentioned in the commit message.
I think for some scanfs ignoring trailing stuff in the string should
just be ignore and in other places trailing stuff should be considered
as error.
> Other than that, this patch looks fine.
>
Matthias
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list