On 17.03.2015 15:25, Maxim Nestratov wrote:
> 17.03.2015 17:15, Michal Privoznik пишет:
>> On 13.03.2015 16:52, Maxim Nestratov wrote:
>>> Signed-off-by: Maxim Nestratov <mnestratov@xxxxxxxxxxxxx>
>>> ---
>>> src/parallels/parallels_network.c | 6 +++---
>>> src/parallels/parallels_sdk.c | 6 +++---
>>> src/parallels/parallels_utils.h | 8 +++++++-
>>> 3 files changed, 13 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/parallels/parallels_network.c
>>> b/src/parallels/parallels_network.c
>>> index 1d3b694..bb7ec5e 100644
>>> --- a/src/parallels/parallels_network.c
>>> +++ b/src/parallels/parallels_network.c
>>> @@ -211,12 +211,12 @@ parallelsLoadNetwork(parallelsConnPtr privconn,
>>> virJSONValuePtr jobj)
>>> goto cleanup;
>>> }
>>> - if (STREQ(tmp, "bridged")) {
>>> + if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
>>> def->forward.type = VIR_NETWORK_FORWARD_BRIDGE;
>>> if (parallelsGetBridgedNetInfo(def, jobj) < 0)
>>> goto cleanup;
>>> - } else if (STREQ(tmp, "host-only")) {
>>> + } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
>>> def->forward.type = VIR_NETWORK_FORWARD_NONE;
>>>
>> How about introducing 'int parallelsNetworkTypeFromString(const char *)'
>> using our ENUM_DECL and ENUM_IMPL macros? This code could then be turned
>> into:
>>
>> if ((def->forward.type = parallelNetworkTypeFromString(tmp)) < 0) {
>> parallelsParseError();
>> goto cleanup;
>> }
>>
>> switch((virNetworkForwardType) def->forward.type) {
>> case VIR_NETWORK_FORWARD_BRIDGE:
>> if (parallelsGetBridgedNetInfo(def, jobj) < 0)
>> goto cleanup;
>> break;
>> case VIR_NETWORK_FORWARD_NONE:
>> if (parallelsGetHostOnlyNetInfo(def, def->name) < 0)
>> goto cleanup;
>> ...
>> }
>>
>> I find it more future proof then STREQ() spaghetti.
>>
>> Michal
> Ok. Makes sence. I'll do this in the next series version.
>
D'oh. Now that I'm going through the code again, this patch actually
makes sense and my suggestion doesn't. The problem is, initially I
thought that @tmp holds network type in string, while in fact it holds a
network's name. And it doesn't make much sense to create an enum for
that. Although, other patches in the series need some discussion before
I can push them.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list