On 2 October 2015 at 19:20, Sair, Umair <umair_s...@mentor.com> wrote: > If I am understanding correct, you are saying that we should set > addr->has_ipv6 and addr->has_ipv4 to true in any case, then in my opinion we > should simply ignore the value of addr->has_ipv* while evaluating the value > of ipv4 and ipv6 variables in inet_addr_to_opts (bool ipv4 = addr->ipv4; bool > ipv6 = addr->ipv6). And the "if (!ipv4 || !ipv6)" condition seems really > strange to me.
No, the has_ipv* flags are important. The major use of this structure is as the internal representation of a JSON-like data structure. This always has a "has_foo" bool field for any optional "foo" data field. So the three states foo can have are: * has_foo = false => foo not specified * has_foo = true, foo = false => foo specified as false * has_foo = true, foo = true => foo specified as true (Eventually somebody will clean this code up so that rather than "create an InetSocketAddress*, then convert it to QemuOpts, then do the actual operation based on the QemuOpts" we will just do "create InetSocketAddress*, then do the operation based on the InetSocketAddress*". That's why we have this indirection in the code, which we want to preserve. Unfortunately first we have to convert a lot of legacy callers that directly call the "do op based on QemuOpts" functions so they call "do op based on InetSocketAddress*".) I agree about the (!ipv4 || !ipv6) condition though. The three states I listed above ought to correspond to "qemu_opt not set", "qemu_opt set to false" and "qemu_opt set to true", -- PMM