Re: [libvirt] [PATCH 10/12] src: add check for termios.h and conditionally include it
On Tue, Jan 14, 2020 at 04:12:15PM +0100, Pavel Hrdina wrote:
On Fri, Jan 10, 2020 at 03:41:14PM +0000, Daniel P. Berrangé wrote:
> The GNULIB termios module ensures termios.h exists, but
> this is not neccessary and libvirt doesn't use any of its
> functionality on platforms where it is missing. It is
> thus sufficient to conditonallyinclude termios.h
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> configure.ac | 1 +
> src/util/virfdstream.c | 4 +++-
> src/util/virfile.c | 4 +++-
> src/util/virutil.c | 1 -
> tools/virsh.h | 1 -
> tools/vsh.h | 4 +++-
> 6 files changed, 10 insertions(+), 5 deletions(-)
This makes the code guarded by HAVE_CFMAKERAW or by !WIN32 and the
header by HAVE_TERMIOS_H which makes things more confusing.
Should we take the opportunity to unify it?
Yeah we have alot of this kind of thing, where we could eliminate a
whole bunch of HAVE_XXXX, in favour of just using #ifdef WIN32.
vircommand.c is a particular case in point where essentially the
whole file should be ifdef WIN32.
Taking this kind of approach would eliminate the need to check
some things in configure.ac and thus cut down what we need for
meson too.
Essentially if, given our supported platform list, we can assume
existance of a header / function on every UNIX platform we care
about, then I think it is reasonable to use ifdef WIN32 for the
exception on Windows.
IOW we only need HAVE_XXX checks if there is variance between UNIX
platforms we need to identify.
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 :|