Re: [PATCHv2 7/9] conf: simplify Buffer Indentation in virDomainNetDefFormat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This looks good to me.

Acked-by: Kyle Mestery <kmestery@xxxxxxxxx>

On Aug 14, 2012, at 2:04 AM, Laine Stump wrote:

> This function has several calls to increase the buffer indent by 6,
> then decrease it again, then increase, then decrease. Additionally,
> there were several printfs that had 6 spaces at the beginning of the
> line.
> 
> virDomainActualNetDefFormat, which is called by virDomainNetDefFormat,
> had similar ugliness.
> 
> This patch changes both functions to just increase the indent at the
> beginning, decrease it at (well, just before*) the end, and remove all
> of the occurences of 6/8 spaces at the beginning of lines.
> 
> *The indent had to be reset before the end of the function because
> virDomainDeviceInfoFormat assumes a 0 indent and is called from many
> other places, and I didn't want to do an overhaul of every caller of
> that function. A separate patch to switch all of domain_conf.c would
> be a useful exercise, but my current goal is unrelated to that, so
> I'll leave it for another day.
> ---
> src/conf/domain_conf.c | 73 +++++++++++++++++++++++---------------------------
> 1 file changed, 33 insertions(+), 40 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 5990634..8f1f244 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -11560,21 +11560,22 @@ virDomainActualNetDefFormat(virBufferPtr buf,
>         return -1;
>     }
> 
> -    virBufferAsprintf(buf, "      <actual type='%s'", type);
> +    virBufferAsprintf(buf, "<actual type='%s'", type);
>     if (def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV &&
>         def->data.hostdev.def.managed) {
>         virBufferAddLit(buf, " managed='yes'");
>     }
>     virBufferAddLit(buf, ">\n");
> 
> +    virBufferAdjustIndent(buf, 2);
>     switch (def->type) {
>     case VIR_DOMAIN_NET_TYPE_BRIDGE:
> -        virBufferEscapeString(buf, "        <source bridge='%s'/>\n",
> +        virBufferEscapeString(buf, "<source bridge='%s'/>\n",
>                               def->data.bridge.brname);
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_DIRECT:
> -        virBufferAddLit(buf, "        <source");
> +        virBufferAddLit(buf, "<source");
>         if (def->data.direct.linkdev)
>             virBufferEscapeString(buf, " dev='%s'",
>                                   def->data.direct.linkdev);
> @@ -11590,12 +11591,10 @@ virDomainActualNetDefFormat(virBufferPtr buf,
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> -        virBufferAdjustIndent(buf, 8);
>         if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def,
>                                          flags, true) < 0) {
>             return -1;
>         }
> -        virBufferAdjustIndent(buf, -8);
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_NETWORK:
> @@ -11606,14 +11605,13 @@ virDomainActualNetDefFormat(virBufferPtr buf,
>         return -1;
>     }
> 
> -    virBufferAdjustIndent(buf, 8);
>     if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
>        return -1;
>     if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
>         return -1;
> -    virBufferAdjustIndent(buf, -8);
> 
> -    virBufferAddLit(buf, "      </actual>\n");
> +    virBufferAdjustIndent(buf, -2);
> +    virBufferAddLit(buf, "</actual>\n");
>     return 0;
> }
> 
> @@ -11637,14 +11635,15 @@ virDomainNetDefFormat(virBufferPtr buf,
>     }
>     virBufferAddLit(buf, ">\n");
> 
> +    virBufferAdjustIndent(buf, 6);
>     virBufferAsprintf(buf,
> -                      "      <mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n",
> +                      "<mac address='%02x:%02x:%02x:%02x:%02x:%02x'/>\n",
>                       def->mac.addr[0], def->mac.addr[1], def->mac.addr[2],
>                       def->mac.addr[3], def->mac.addr[4], def->mac.addr[5]);
> 
>     switch (def->type) {
>     case VIR_DOMAIN_NET_TYPE_NETWORK:
> -        virBufferEscapeString(buf, "      <source network='%s'",
> +        virBufferEscapeString(buf, "<source network='%s'",
>                               def->data.network.name);
>         virBufferEscapeString(buf, " portgroup='%s'",
>                               def->data.network.portgroup);
> @@ -11655,39 +11654,41 @@ virDomainNetDefFormat(virBufferPtr buf,
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_ETHERNET:
> -        virBufferEscapeString(buf, "      <source dev='%s'/>\n",
> +        virBufferEscapeString(buf, "<source dev='%s'/>\n",
>                               def->data.ethernet.dev);
>         if (def->data.ethernet.ipaddr)
> -            virBufferAsprintf(buf, "      <ip address='%s'/>\n",
> +            virBufferAsprintf(buf, "<ip address='%s'/>\n",
>                               def->data.ethernet.ipaddr);
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_BRIDGE:
> -        virBufferEscapeString(buf, "      <source bridge='%s'/>\n",
> +        virBufferEscapeString(buf, "<source bridge='%s'/>\n",
>                               def->data.bridge.brname);
> -        if (def->data.bridge.ipaddr)
> -            virBufferAsprintf(buf, "      <ip address='%s'/>\n",
> +        if (def->data.bridge.ipaddr) {
> +            virBufferAsprintf(buf, "<ip address='%s'/>\n",
>                               def->data.bridge.ipaddr);
> +        }
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_SERVER:
>     case VIR_DOMAIN_NET_TYPE_CLIENT:
>     case VIR_DOMAIN_NET_TYPE_MCAST:
> -        if (def->data.socket.address)
> -            virBufferAsprintf(buf, "      <source address='%s' port='%d'/>\n",
> +        if (def->data.socket.address) {
> +            virBufferAsprintf(buf, "<source address='%s' port='%d'/>\n",
>                               def->data.socket.address, def->data.socket.port);
> -        else
> -            virBufferAsprintf(buf, "      <source port='%d'/>\n",
> +        } else {
> +            virBufferAsprintf(buf, "<source port='%d'/>\n",
>                               def->data.socket.port);
> +        }
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_INTERNAL:
> -        virBufferEscapeString(buf, "      <source name='%s'/>\n",
> +        virBufferEscapeString(buf, "<source name='%s'/>\n",
>                               def->data.internal.name);
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_DIRECT:
> -        virBufferEscapeString(buf, "      <source dev='%s'",
> +        virBufferEscapeString(buf, "<source dev='%s'",
>                               def->data.direct.linkdev);
>         virBufferAsprintf(buf, " mode='%s'",
>                           virNetDevMacVLanModeTypeToString(def->data.direct.mode));
> @@ -11695,12 +11696,10 @@ virDomainNetDefFormat(virBufferPtr buf,
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> -        virBufferAdjustIndent(buf, 6);
>         if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def,
>                                          flags, true) < 0) {
>             return -1;
>         }
> -        virBufferAdjustIndent(buf, -6);
>         break;
> 
>     case VIR_DOMAIN_NET_TYPE_USER:
> @@ -11708,26 +11707,22 @@ virDomainNetDefFormat(virBufferPtr buf,
>         break;
>     }
> 
> -    virBufferAdjustIndent(buf, 6);
>     if (virNetDevVPortProfileFormat(def->virtPortProfile, buf) < 0)
>        return -1;
> -    virBufferAdjustIndent(buf, -6);
> -
> -    virBufferEscapeString(buf, "      <script path='%s'/>\n",
> +    virBufferEscapeString(buf, "<script path='%s'/>\n",
>                           def->script);
>     if (def->ifname &&
>         !((flags & VIR_DOMAIN_XML_INACTIVE) &&
>           (STRPREFIX(def->ifname, VIR_NET_GENERATED_PREFIX)))) {
>         /* Skip auto-generated target names for inactive config. */
> -        virBufferEscapeString(buf, "      <target dev='%s'/>\n",
> -                              def->ifname);
> +        virBufferEscapeString(buf, "<target dev='%s'/>\n", def->ifname);
>     }
>     if (def->model) {
> -        virBufferEscapeString(buf, "      <model type='%s'/>\n",
> +        virBufferEscapeString(buf, "<model type='%s'/>\n",
>                               def->model);
>         if (STREQ(def->model, "virtio") &&
>             (def->driver.virtio.name || def->driver.virtio.txmode)) {
> -            virBufferAddLit(buf, "      <driver");
> +            virBufferAddLit(buf, "<driver");
>             if (def->driver.virtio.name) {
>                 virBufferAsprintf(buf, " name='%s'",
>                                   virDomainNetBackendTypeToString(def->driver.virtio.name));
> @@ -11748,27 +11743,25 @@ virDomainNetDefFormat(virBufferPtr buf,
>         }
>     }
>     if (def->filter) {
> -        virBufferAdjustIndent(buf, 6);
>         if (virNWFilterFormatParamAttributes(buf, def->filterparams,
>                                              def->filter) < 0)
>             return -1;
> -        virBufferAdjustIndent(buf, -6);
>     }
> 
>     if (def->tune.sndbuf_specified) {
> -        virBufferAddLit(buf,   "      <tune>\n");
> -        virBufferAsprintf(buf, "        <sndbuf>%lu</sndbuf>\n",
> -                          def->tune.sndbuf);
> -        virBufferAddLit(buf,   "      </tune>\n");
> +        virBufferAddLit(buf,   "<tune>\n");
> +        virBufferAsprintf(buf, "  <sndbuf>%lu</sndbuf>\n", def->tune.sndbuf);
> +        virBufferAddLit(buf,   "</tune>\n");
>     }
> 
> -    if (def->linkstate)
> -        virBufferAsprintf(buf, "      <link state='%s'/>\n",
> +    if (def->linkstate) {
> +        virBufferAsprintf(buf, "<link state='%s'/>\n",
>                           virDomainNetInterfaceLinkStateTypeToString(def->linkstate));
> +    }
> 
> -    virBufferAdjustIndent(buf, 6);
>     if (virNetDevBandwidthFormat(def->bandwidth, buf) < 0)
>         return -1;
> +
>     virBufferAdjustIndent(buf, -6);
> 
>     if (virDomainDeviceInfoFormat(buf, &def->info,
> -- 
> 1.7.11.2
> 
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]