Re: [libvirt] [PATCH 02/10] Replace sscanf in nwfilter rule parsing
2010/3/30 Jim Meyering <jim(a)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