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

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



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 :|


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