1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH v2 0/5] Misc cleanups for internal.h & fix tests with CLang
  4. View patch

[libvirt] [PATCH v2 3/5] Remove network constants out of internal.h

Daniel P. Berrange posted 5 patches 7 years, 10 months ago
[libvirt] [PATCH v2 3/5] Remove network constants out of internal.h
Posted by Daniel P. Berrange 7 years, 10 months ago 
The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
constants are only used by a handful of files, so are better
kept in virsocketaddr.h

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 src/internal.h                         | 16 ----------------
 src/libxl/libxl_conf.c                 |  1 +
 src/nwfilter/nwfilter_dhcpsnoop.c      |  1 +
 src/nwfilter/nwfilter_gentech_driver.c |  1 +
 src/qemu/qemu_conf.c                   |  1 +
 src/util/virsocketaddr.h               | 16 ++++++++++++++++
 src/util/virutil.c                     |  1 +
 src/vz/vz_sdk.c                        |  1 +
 8 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/internal.h b/src/internal.h
index 69dff41..2b8cc09 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -65,22 +65,6 @@
 # include "ignore-value.h"
 # include "count-leading-zeros.h"
 
-/* On architectures which lack these limits, define them (ie. Cygwin).
- * Note that the libvirt code should be robust enough to handle the
- * case where actual value is longer than these limits (eg. by setting
- * length correctly in second argument to gethostname and by always
- * using strncpy instead of strcpy).
- */
-# ifndef HOST_NAME_MAX
-#  define HOST_NAME_MAX 256
-# endif
-
-# ifndef INET_ADDRSTRLEN
-#  define INET_ADDRSTRLEN 16
-# endif
-
-# define VIR_LOOPBACK_IPV4_ADDR "127.0.0.1"
-
 /* String equality tests, suggested by Jim Meyering. */
 # define STREQ(a, b) (strcmp(a, b) == 0)
 # define STRCASEEQ(a, b) (c_strcasecmp(a, b) == 0)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 938e09d..a85bc71 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -42,6 +42,7 @@
 #include "viralloc.h"
 #include "viruuid.h"
 #include "vircommand.h"
+#include "virsocketaddr.h"
 #include "libxl_domain.h"
 #include "libxl_conf.h"
 #include "libxl_utils.h"
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index 702abe1..4436e39 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -65,6 +65,7 @@
 #include "virnetdev.h"
 #include "virfile.h"
 #include "viratomic.h"
+#include "virsocketaddr.h"
 #include "virthreadpool.h"
 #include "configmake.h"
 #include "virtime.h"
diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 82e20de..3d809fb 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -37,6 +37,7 @@
 #include "nwfilter_learnipaddr.h"
 #include "virnetdev.h"
 #include "datatypes.h"
+#include "virsocketaddr.h"
 #include "virstring.h"
 
 #define VIR_FROM_THIS VIR_FROM_NWFILTER
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 73c33d6..a65c92a 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -49,6 +49,7 @@
 #include "cpu/cpu.h"
 #include "domain_nwfilter.h"
 #include "virfile.h"
+#include "virsocketaddr.h"
 #include "virstring.h"
 #include "viratomic.h"
 #include "storage_conf.h"
diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
index 43a3706..ae76166 100644
--- a/src/util/virsocketaddr.h
+++ b/src/util/virsocketaddr.h
@@ -32,6 +32,22 @@
 #  include <sys/un.h>
 # endif
 
+/* On architectures which lack these limits, define them (ie. Cygwin).
+ * Note that the libvirt code should be robust enough to handle the
+ * case where actual value is longer than these limits (eg. by setting
+ * length correctly in second argument to gethostname and by always
+ * using strncpy instead of strcpy).
+ */
+# ifndef HOST_NAME_MAX
+#  define HOST_NAME_MAX 256
+# endif
+
+# ifndef INET_ADDRSTRLEN
+#  define INET_ADDRSTRLEN 16
+# endif
+
+# define VIR_LOOPBACK_IPV4_ADDR "127.0.0.1"
+
 typedef struct {
     union {
         struct sockaddr sa;
diff --git a/src/util/virutil.c b/src/util/virutil.c
index e4de4ca..88fff64 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -81,6 +81,7 @@
 #include "virprocess.h"
 #include "virstring.h"
 #include "virutil.h"
+#include "virsocketaddr.h"
 
 verify(sizeof(gid_t) <= sizeof(unsigned int) &&
        sizeof(uid_t) <= sizeof(unsigned int));
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index 0aa1a30..c5f11a4 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -31,6 +31,7 @@
 #include "domain_conf.h"
 #include "virtime.h"
 #include "virhostcpu.h"
+#include "virsocketaddr.h"
 
 #include "storage/storage_driver.h"
 #include "vz_sdk.h"
-- 
2.9.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] Remove network constants out of internal.h
Posted by Peter Krempa 7 years, 10 months ago 
On Wed, Jul 05, 2017 at 12:58:49 +0100, Daniel Berrange wrote:
> The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
> constants are only used by a handful of files, so are better
> kept in virsocketaddr.h
> 
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  src/internal.h                         | 16 ----------------
>  src/libxl/libxl_conf.c                 |  1 +
>  src/nwfilter/nwfilter_dhcpsnoop.c      |  1 +
>  src/nwfilter/nwfilter_gentech_driver.c |  1 +
>  src/qemu/qemu_conf.c                   |  1 +
>  src/util/virsocketaddr.h               | 16 ++++++++++++++++
>  src/util/virutil.c                     |  1 +
>  src/vz/vz_sdk.c                        |  1 +
>  8 files changed, 22 insertions(+), 16 deletions(-)
> 

[...]

> diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
> index 43a3706..ae76166 100644
> --- a/src/util/virsocketaddr.h
> +++ b/src/util/virsocketaddr.h
> @@ -32,6 +32,22 @@
>  #  include <sys/un.h>
>  # endif
>  
> +/* On architectures which lack these limits, define them (ie. Cygwin).
> + * Note that the libvirt code should be robust enough to handle the
> + * case where actual value is longer than these limits (eg. by setting
> + * length correctly in second argument to gethostname and by always
> + * using strncpy instead of strcpy).
> + */
> +# ifndef HOST_NAME_MAX
> +#  define HOST_NAME_MAX 256

I'm not entirely convinced that this semantically belongs to
virsocketaddr.h

At least in the virlog.c usage case it is not used in any way in regards
to network communication.

> +# endif
> +
> +# ifndef INET_ADDRSTRLEN
> +#  define INET_ADDRSTRLEN 16
> +# endif
> +
> +# define VIR_LOOPBACK_IPV4_ADDR "127.0.0.1"
> +
>  typedef struct {
>      union {
>          struct sockaddr sa;

ACK to the rest though.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] Remove network constants out of internal.h
Posted by Daniel P. Berrange 7 years, 10 months ago 
On Mon, Jul 10, 2017 at 01:01:26PM +0200, Peter Krempa wrote:
> On Wed, Jul 05, 2017 at 12:58:49 +0100, Daniel Berrange wrote:
> > The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
> > constants are only used by a handful of files, so are better
> > kept in virsocketaddr.h
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  src/internal.h                         | 16 ----------------
> >  src/libxl/libxl_conf.c                 |  1 +
> >  src/nwfilter/nwfilter_dhcpsnoop.c      |  1 +
> >  src/nwfilter/nwfilter_gentech_driver.c |  1 +
> >  src/qemu/qemu_conf.c                   |  1 +
> >  src/util/virsocketaddr.h               | 16 ++++++++++++++++
> >  src/util/virutil.c                     |  1 +
> >  src/vz/vz_sdk.c                        |  1 +
> >  8 files changed, 22 insertions(+), 16 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
> > index 43a3706..ae76166 100644
> > --- a/src/util/virsocketaddr.h
> > +++ b/src/util/virsocketaddr.h
> > @@ -32,6 +32,22 @@
> >  #  include <sys/un.h>
> >  # endif
> >  
> > +/* On architectures which lack these limits, define them (ie. Cygwin).
> > + * Note that the libvirt code should be robust enough to handle the
> > + * case where actual value is longer than these limits (eg. by setting
> > + * length correctly in second argument to gethostname and by always
> > + * using strncpy instead of strcpy).
> > + */
> > +# ifndef HOST_NAME_MAX
> > +#  define HOST_NAME_MAX 256
> 
> I'm not entirely convinced that this semantically belongs to
> virsocketaddr.h
> 
> At least in the virlog.c usage case it is not used in any way in regards
> to network communication.

The constant is used to define a hostname string buffer, which is then
used with the gethostname() system call. That's network related IMHO.

> ACK to the rest though.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] Remove network constants out of internal.h
Posted by Peter Krempa 7 years, 10 months ago 
On Mon, Jul 10, 2017 at 16:07:08 +0100, Daniel Berrange wrote:
> On Mon, Jul 10, 2017 at 01:01:26PM +0200, Peter Krempa wrote:
> > On Wed, Jul 05, 2017 at 12:58:49 +0100, Daniel Berrange wrote:
> > > The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
> > > constants are only used by a handful of files, so are better
> > > kept in virsocketaddr.h
> > > 
> > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > > ---
> > >  src/internal.h                         | 16 ----------------
> > >  src/libxl/libxl_conf.c                 |  1 +
> > >  src/nwfilter/nwfilter_dhcpsnoop.c      |  1 +
> > >  src/nwfilter/nwfilter_gentech_driver.c |  1 +
> > >  src/qemu/qemu_conf.c                   |  1 +
> > >  src/util/virsocketaddr.h               | 16 ++++++++++++++++
> > >  src/util/virutil.c                     |  1 +
> > >  src/vz/vz_sdk.c                        |  1 +
> > >  8 files changed, 22 insertions(+), 16 deletions(-)
> > > 
> > 
> > [...]
> > 
> > > diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
> > > index 43a3706..ae76166 100644
> > > --- a/src/util/virsocketaddr.h
> > > +++ b/src/util/virsocketaddr.h
> > > @@ -32,6 +32,22 @@
> > >  #  include <sys/un.h>
> > >  # endif
> > >  
> > > +/* On architectures which lack these limits, define them (ie. Cygwin).
> > > + * Note that the libvirt code should be robust enough to handle the
> > > + * case where actual value is longer than these limits (eg. by setting
> > > + * length correctly in second argument to gethostname and by always
> > > + * using strncpy instead of strcpy).
> > > + */
> > > +# ifndef HOST_NAME_MAX
> > > +#  define HOST_NAME_MAX 256
> > 
> > I'm not entirely convinced that this semantically belongs to
> > virsocketaddr.h
> > 
> > At least in the virlog.c usage case it is not used in any way in regards
> > to network communication.
> 
> The constant is used to define a hostname string buffer, which is then
> used with the gethostname() system call. That's network related IMHO.

I find it weird, that it gets defined in virsocketaddr.h, but the only
usage is in src/util/virutil.c thus it needs cross-inclusions which were
not in place prior to the move.

Should perhaps virGetHostnameImpl and friends be moved too in that case?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] Remove network constants out of internal.h
Posted by Daniel P. Berrange 7 years, 10 months ago 
On Tue, Jul 11, 2017 at 02:27:10PM +0200, Peter Krempa wrote:
> On Mon, Jul 10, 2017 at 16:07:08 +0100, Daniel Berrange wrote:
> > On Mon, Jul 10, 2017 at 01:01:26PM +0200, Peter Krempa wrote:
> > > On Wed, Jul 05, 2017 at 12:58:49 +0100, Daniel Berrange wrote:
> > > > The HOST_NAME_MAX, INET_ADDRSTRLEN and VIR_LOOPBACK_IPV4_ADDR
> > > > constants are only used by a handful of files, so are better
> > > > kept in virsocketaddr.h
> > > > 
> > > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > > > ---
> > > >  src/internal.h                         | 16 ----------------
> > > >  src/libxl/libxl_conf.c                 |  1 +
> > > >  src/nwfilter/nwfilter_dhcpsnoop.c      |  1 +
> > > >  src/nwfilter/nwfilter_gentech_driver.c |  1 +
> > > >  src/qemu/qemu_conf.c                   |  1 +
> > > >  src/util/virsocketaddr.h               | 16 ++++++++++++++++
> > > >  src/util/virutil.c                     |  1 +
> > > >  src/vz/vz_sdk.c                        |  1 +
> > > >  8 files changed, 22 insertions(+), 16 deletions(-)
> > > > 
> > > 
> > > [...]
> > > 
> > > > diff --git a/src/util/virsocketaddr.h b/src/util/virsocketaddr.h
> > > > index 43a3706..ae76166 100644
> > > > --- a/src/util/virsocketaddr.h
> > > > +++ b/src/util/virsocketaddr.h
> > > > @@ -32,6 +32,22 @@
> > > >  #  include <sys/un.h>
> > > >  # endif
> > > >  
> > > > +/* On architectures which lack these limits, define them (ie. Cygwin).
> > > > + * Note that the libvirt code should be robust enough to handle the
> > > > + * case where actual value is longer than these limits (eg. by setting
> > > > + * length correctly in second argument to gethostname and by always
> > > > + * using strncpy instead of strcpy).
> > > > + */
> > > > +# ifndef HOST_NAME_MAX
> > > > +#  define HOST_NAME_MAX 256
> > > 
> > > I'm not entirely convinced that this semantically belongs to
> > > virsocketaddr.h
> > > 
> > > At least in the virlog.c usage case it is not used in any way in regards
> > > to network communication.
> > 
> > The constant is used to define a hostname string buffer, which is then
> > used with the gethostname() system call. That's network related IMHO.
> 
> I find it weird, that it gets defined in virsocketaddr.h, but the only
> usage is in src/util/virutil.c thus it needs cross-inclusions which were
> not in place prior to the move.
> 
> Should perhaps virGetHostnameImpl and friends be moved too in that case?

Lets just move HOST_NAME_MAX out of the headers entirely and just pyut it
in virutil.c at the virGetHostname method definition.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] Remove network constants out of internal.h
Posted by Peter Krempa 7 years, 10 months ago 
On Tue, Jul 11, 2017 at 13:38:56 +0100, Daniel Berrange wrote:
> On Tue, Jul 11, 2017 at 02:27:10PM +0200, Peter Krempa wrote:
> > On Mon, Jul 10, 2017 at 16:07:08 +0100, Daniel Berrange wrote:
> > > On Mon, Jul 10, 2017 at 01:01:26PM +0200, Peter Krempa wrote:
> > > > On Wed, Jul 05, 2017 at 12:58:49 +0100, Daniel Berrange wrote:

[...]

> > > > > +# ifndef HOST_NAME_MAX
> > > > > +#  define HOST_NAME_MAX 256
> > > > 
> > > > I'm not entirely convinced that this semantically belongs to
> > > > virsocketaddr.h
> > > > 
> > > > At least in the virlog.c usage case it is not used in any way in regards
> > > > to network communication.
> > > 
> > > The constant is used to define a hostname string buffer, which is then
> > > used with the gethostname() system call. That's network related IMHO.
> > 
> > I find it weird, that it gets defined in virsocketaddr.h, but the only
> > usage is in src/util/virutil.c thus it needs cross-inclusions which were
> > not in place prior to the move.
> > 
> > Should perhaps virGetHostnameImpl and friends be moved too in that case?
> 
> Lets just move HOST_NAME_MAX out of the headers entirely and just pyut it
> in virutil.c at the virGetHostname method definition.

Yep, that's reasonable. ACK to that.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.