Re: [Qemu-devel] [PATCH v7 2/5] sockets: Change inet_parse() to accept address specification without port

[Paolo Bonzini <pbonzini@redhat.com>]

Il 18/09/2012 15:22, Kevin Wolf ha scritto:
> Am 17.09.2012 17:23, schrieb Bharata B Rao:
>> sockets: Change inet_parse() to accept address specification without port
>>
>> From: Bharata B Rao <bharata@linux.vnet.ibm.com>
>>
>> inet_parse() expects address:port. Change it to work without explicit port
>> specification. In addition, don't depend solely on the return value of
> 
> Things like "in addition" in a commit message are almost always a sign
> that the patch should be split in two.
> 
>> sscanf but also consider the value obtained for %n directive used in sscanf.
>> This ensures that the scanning of malformed inet address isn't flagged as
>> success.
> 
> Can you give an example string that would be falsely accepted? To me the
> old checks look fine (even though the new ones are a little bit easier
> to read, so even if they don't fix anything, they might be worth doing).

"localhost" would fail to be parsed:

-        if (2 != sscanf(str,"%64[^:]:%32[^,]%n",addr,port,&pos)) {
+        ret = sscanf(str, "%64[^:]%n:%32[^,]%n", addr, &addr_pos,
+            port, &port_pos);
+        if (addr_pos == -1 || ret == EOF) {

because the : in the format string would not match and sscanf would
return 1.

However, is it correct to set the port unconditionally to an empty
string?  Your usecase makes sense, but perhaps the default port be
passed as an extra parameter to inet_parse instead.

Paolo

> Anyway, it does look correct.
> 
> Kevin
> 
>