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

Re: [libvirt] [PATCH 06/11] Expand virSocketFormat to be more flexible



On 10/21/2010 12:17 PM, Daniel P. Berrange wrote:
The getnameinfo() function is more flexible than inet_ntop()
avoiding the need to if/else the code based on socket family.
Also make it support UNIX socket addrs and allow inclusion
of a port (service) address. Finally do proper error reporting
via normal APIs.

* src/conf/domain_conf.c, src/nwfilter/nwfilter_ebiptables_driver.c,
   src/qemu/qemu_conf.c: Fix error handling with virSocketFormat
* src/util/network.c: Rewrite virSocketFormat to use getnameinfo
   and cope with UNIX socket addrs.
---
  po/POTFILES.in                            |    1 +
  src/conf/domain_conf.c                    |    5 +--
  src/libvirt_private.syms                  |    1 +
  src/nwfilter/nwfilter_ebiptables_driver.c |   12 +----
  src/qemu/qemu_conf.c                      |    2 +
  src/util/network.c                        |   82 ++++++++++++++++++++++-------
  src/util/network.h                        |    5 ++
  7 files changed, 75 insertions(+), 33 deletions(-)

+/*
+ * virSocketFormatAddr:

s/Addr:/AddrFull:/

+ * @addr: an initialized virSocketAddrPtr
+ * @withService: if true, then service info is appended
+ * @separator: separator between hostname&  service.

Must be non-NULL if withService is true, and ignored when withService is false? In which case, is it merely sufficient to provide a non-NULL string to request withService, in which point the second parameter is redundant?

+ *
+ * Returns a string representation of the given address
+ * Returns NULL on any error
+ * Caller must free the returned string
+ */
+char *
+virSocketFormatAddrFull(virSocketAddrPtr addr,
+                        bool withService,
+                        const char *separator)
+{
+    char host[NI_MAXHOST], port[NI_MAXSERV];
+    char *addrstr;
+    int err;

-    else if (addr->data.stor.ss_family == AF_INET6) {
-        outlen = INET6_ADDRSTRLEN;
-        inaddr =&addr->data.inet6.sin6_addr;
+    if (addr == NULL) {
+        virSocketError(VIR_ERR_INVALID_ARG, _("Missing address"));
+        return NULL;
      }

-    else {
-        return NULL;
+    /* Short-circuit since getnameinfo doesn't work
+     * nicely for UNIX sockets */
+    if (addr->data.sa.sa_family == AF_UNIX) {
+        if (withService) {
+            if (virAsprintf(&addrstr, "127.0.0.1%s0",
+                            separator ? separator : ":")<  0)

Oh, I see - separator defaults to ":" if withService is true but separator is NULL. Still, is the three-argument version buying us anything over a two-argument version where the caller must supply ":" rather than relying on a default?

ACK if you decide 3 arguments is worth it, and with the doc typo fixed.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


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