2010/3/30 Jim Meyering <jim@meyering.net>:
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