2:13 p.m.
Fixes the following warning:
util/virterror.c:1242:31: warning: declaration of 'errcode' shadows a global
declaration [-Wshadow]
---
src/util/virterror.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virterror.c b/src/util/virterror.c
index ff44a57..85eec8d 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -1228,7 +1228,7 @@ virErrorMsg(virErrorNumber error, const char *info)
* virReportErrorHelper:
*
* @domcode: the virErrorDomain indicating where it's coming from
- * @errcode: the virErrorNumber code for the error
+ * @errorcode: the virErrorNumber code for the error
* @filename: Source file error is dispatched from
* @funcname: Function error is dispatched from
* @linenr: Line number error is dispatched from
@@ -1239,7 +1239,7 @@ virErrorMsg(virErrorNumber error, const char *info)
* ReportError
*/
void virReportErrorHelper(int domcode,
- int errcode,
+ int errorcode,
const char *filename,
const char *funcname,
size_t linenr,
@@ -1258,9 +1258,9 @@ void virReportErrorHelper(int domcode,
errorMessage[0] = '\0';
}
- virerr = virErrorMsg(errcode, (errorMessage[0] ? errorMessage : NULL));
+ virerr = virErrorMsg(errorcode, (errorMessage[0] ? errorMessage : NULL));
virRaiseErrorFull(filename, funcname, linenr,
- domcode, errcode, VIR_ERR_ERROR,
+ domcode, errorcode, VIR_ERR_ERROR,
virerr, errorMessage, NULL,
-1, -1, virerr, errorMessage);
errno = save_errno;
--
1.7.7.5
2:13 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
windows.h is included by threads.h.
winsock2.h should be included before.
Avoid the following warning:
In file included from ../gnulib/lib/unistd.h:51:0,
from ../src/util/util.h:30,
from rpc/virkeepalive.c:29:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2: warning: #warning Please
include winsock2.h before windows.h [-Wcpp]
---
src/rpc/virkeepalive.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c
index 06b8e63..f345b85 100644
--- a/src/rpc/virkeepalive.c
+++ b/src/rpc/virkeepalive.c
@@ -22,6 +22,10 @@
#include <config.h>
+#ifdef HAVE_WINSOCK2_H
+# include <winsock2.h>
+#endif
+
#include "memory.h"
#include "threads.h"
#include "virfile.h"
--
1.7.7.5
4 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
windows.h is included by threads.h.
winsock2.h should be included before.
Avoid the following warning:
In file included from ../gnulib/lib/unistd.h:51:0,
from ../src/util/util.h:30,
from rpc/virkeepalive.c:29:
/usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2: warning: #warning Please
include winsock2.h before windows.h [-Wcpp]
---
src/rpc/virkeepalive.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
NACK. I agree that things should be fixed, but the fix should live in
the problematic "threads.h" inclusion pattern, not in the downstream
virkeepalive.c.
Does this alternate patch work for you? (One of these days, I need to
install the mingw64 cross-compiler, so I can test it myself.)
diff --git i/src/util/threads-win32.h w/src/util/threads-win32.h
index 8109afd..1b23402 100644
--- i/src/util/threads-win32.h
+++ w/src/util/threads-win32.h
@@ -1,7 +1,7 @@
/*
* threads-win32.h basic thread synchronization primitives
*
- * Copyright (C) 2009, 2011 Red Hat, Inc.
+ * Copyright (C) 2009, 2011-2012 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -21,6 +21,9 @@
#include "internal.h"
+#ifdef HAVE_WINSOCK2_H
+# include <winsock2.h>
+#endif
#include <windows.h>
struct virMutex {
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:16 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
----- Mensaje original -----
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
> windows.h is included by threads.h.
> winsock2.h should be included before.
>
> Avoid the following warning:
>
> In file included from ../gnulib/lib/unistd.h:51:0,
> from ../src/util/util.h:30,
> from rpc/virkeepalive.c:29:
> /usr/x86_64-w64-mingw32/sys-root/mingw/include/winsock2.h:15:2:
> warning: #warning Please include winsock2.h before windows.h
> [-Wcpp]
> ---
> src/rpc/virkeepalive.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
NACK. I agree that things should be fixed, but the fix should live
in
the problematic "threads.h" inclusion pattern, not in the downstream
virkeepalive.c.
I agree, but ff we follow that reasoning, windows.h should include winsock2.h,
shouldn't it?
Does this alternate patch work for you? (One of these days, I need
to
install the mingw64 cross-compiler, so I can test it myself.)
Yes, that works too.
Btw, I don't mind testing patches, but using mingw64 is almost a 'yum install'
away :) And you could help to fix the few remaining issues. The most worrying being the
gnulib "stat" module, the rest should be in control.
diff --git i/src/util/threads-win32.h w/src/util/threads-win32.h
index 8109afd..1b23402 100644
--- i/src/util/threads-win32.h
+++ w/src/util/threads-win32.h
@@ -1,7 +1,7 @@
/*
* threads-win32.h basic thread synchronization primitives
*
- * Copyright (C) 2009, 2011 Red Hat, Inc.
+ * Copyright (C) 2009, 2011-2012 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -21,6 +21,9 @@
#include "internal.h"
+#ifdef HAVE_WINSOCK2_H
+# include <winsock2.h>
+#endif
#include <windows.h>
struct virMutex {
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:31 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
On 01/25/2012 03:16 PM, Marc-André Lureau wrote:
> NACK. I agree that things should be fixed, but the fix should
live
> in
> the problematic "threads.h" inclusion pattern, not in the downstream
> virkeepalive.c.
I agree, but ff we follow that reasoning, windows.h should include winsock2.h,
shouldn't it?
If it were POSIX, the answer is yes - all POSIX headers are required to
be idempotent and self-contained, so any header that requires some other
header to be included first, instead of doing the prereq includes under
the hood itself, is broken. And gnulib already does a great job of
fixing the (surprisingly large number of) systems that fail this POSIX
header litmus test.
But we're talking about Windows, so we have to follow Microsoft
conventions. And if Microsoft requires <winsock2.h> to be included
before <windows.h>, then mingw64 is doing the right thing in mimicking
that convention; at which point, we can't insist on "fixing"
<windows.h>. But we _can_ insist on fixing libvirt's use of Windows
headers so that _only_ the windows-specific files have to know about the
dependency ordering constraints, and all other libvirt files can use any
libvirt header without having to think about inherited dependency
constraints. threads-win32.h is our windows-specific header that was
triggering the problem, so that's the point in the chain worth fixing,
so that normal files, like virkeepalive.c don't have to think about "if
I include both "threads.h" and <unistd.h>, am I introducing an ordering
constraint on windows platforms?"
> Does this alternate patch work for you? (One of these days, I
need
> to
> install the mingw64 cross-compiler, so I can test it myself.)
Yes, that works too.
Glad to hear it; I've pushed the alternate patch.
Btw, I don't mind testing patches, but using mingw64 is almost a 'yum
install' away :)
What repo? The mingw64 compiler isn't in Fedora 16 proper.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:42 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
Hi
----- Mensaje original -----
On 01/25/2012 03:16 PM, Marc-André Lureau wrote:
>
> Btw, I don't mind testing patches, but using mingw64 is almost a
> 'yum install' away :)
What repo? The mingw64 compiler isn't in Fedora 16 proper.
The Fedora mingw64 project repo: http://build1.openftd.org/fedora-cross/fedora-cross.repo
(I also build mine one for epel6
http://repos.fedorapeople.org/repos/elmarco/mingw64/epel-mingw64.repo, although it is not
maintained regularly)
6:03 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
On 01/25/2012 03:42 PM, Marc-André Lureau wrote:
Hi
----- Mensaje original -----
> On 01/25/2012 03:16 PM, Marc-André Lureau wrote:
>>
>> Btw, I don't mind testing patches, but using mingw64 is almost a
>> 'yum install' away :)
>
> What repo? The mingw64 compiler isn't in Fedora 16 proper.
>
The Fedora mingw64 project repo: http://build1.openftd.org/fedora-cross/fedora-cross.repo
Thanks; I've got the cross-compiler installed now, so I can at least
play with the gnulib fixes. However, I'm running into:
$ sysroot=$HOME/builder/x86_64-w64-mingw32/sys-root
$ ../configure --build=x86_64-pc-linux --host=x86_64-w64-mingw32 \
--prefix=$sysroot/mingw --enable-compile-warnings=error \
--without-libvirtd --without-python \
PKG_CONFIG_PATH=$sysroot/mingw/lib/pkgconfig
...
checking for xdrmem_create in -lportablexdr... no
checking for library containing xdrmem_create... no
configure: error: Cannot find a XDR library
that is, while there is mingw32-portablexdr in the fedora-cross repo,
there is no mingw64-portablexdr, which chokes up configure. What
configure arguments are you using?
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:15 p.m.
New subject: [libvirt] [PATCH libvirt 2/6] include winsock2.h before windows.h
----- Mensaje original -----
that is, while there is mingw32-portablexdr in the fedora-cross
repo,
there is no mingw64-portablexdr, which chokes up configure. What
configure arguments are you using?
Sorry, I thought the repo was updated more regularly, I know it has been fixed only a few
hours ago in the SVN, so it may take some time to sync. In the mean time, you can compile
the rpm from:
http://svn.openftd.org/svn/fedora_cross/mingw-portablexdr/
For the records, I use:
export
PKG_CONFIG_PATH=/usr/x86_64-w64-mingw32/sys-root/mingw/lib/pkgconfig/:/usr/x86_64-w64-mingw32/sys-root/mingw/share/pkgconfig/:/usr/share/pkgconfig/
./configure --host=x86_64-w64-mingw32 --build=x86_64-unknown-linux-gnu
--target=x86_64-w64-mingw32 --prefix=/usr/x86_64-w64-mingw32/sys-root/mingw
--exec-prefix=/usr/x86_64-w64-mingw32/sys-root/mingw
--bindir=/usr/x86_64-w64-mingw32/sys-root/mingw/bin
--sbindir=/usr/x86_64-w64-mingw32/sys-root/mingw/sbin
--sysconfdir=/usr/x86_64-w64-mingw32/sys-root/mingw/etc
--datadir=/usr/x86_64-w64-mingw32/sys-root/mingw/share
--includedir=/usr/x86_64-w64-mingw32/sys-root/mingw/include
--libdir=/usr/x86_64-w64-mingw32/sys-root/mingw/lib
--libexecdir=/usr/x86_64-w64-mingw32/sys-root/mingw/libexec
--localstatedir=/usr/x86_64-w64-mingw32/sys-root/mingw/var
--sharedstatedir=/usr/x86_64-w64-mingw32/sys-root/mingw/com
--mandir=/usr/x86_64-w64-mingw32/sys-root/mingw/share/man
--infodir=/usr/x86_64-w64-mingw32/sys-root/mingw/share/info --without-xen --without-qemu
--without-openvz --without-lxc --without-vbox --without-xenapi --without-sasl
--without-avahi --without-polkit --without-python --without-libvirtd --without-uml
--without-phyp --without-hyperv --without-vmware --without-netcf --without-audit
--without-dtrace --enable-static
Which is what is used by the mingw-libvirt package.
2:13 p.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
Define PID_FORMAT and fix warnings for mingw64 x86_64 build.
Unfortunately, gnu_printf attribute check expect %lld while normal
printf is PRId64. So one warning remains.
---
src/rpc/virnetsocket.c | 4 ++--
src/util/command.c | 10 +++++-----
src/util/util.h | 8 ++++++++
src/util/virpidfile.c | 6 +++---
4 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 67d33b7..20bee4b 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -114,7 +114,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr,
virNetSocketPtr sock;
int no_slow_start = 1;
- VIR_DEBUG("localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%d",
+ VIR_DEBUG("localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%" PID_FORMAT,
localAddr, remoteAddr,
fd, errfd, pid);
@@ -174,7 +174,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr,
sock->client = isClient;
PROBE(RPC_SOCKET_NEW,
- "sock=%p refs=%d fd=%d errfd=%d pid=%d localAddr=%s, remoteAddr=%s",
+ "sock=%p refs=%d fd=%d errfd=%d pid=%" PID_FORMAT "
localAddr=%s, remoteAddr=%s",
sock, sock->refs, fd, errfd,
pid, NULLSTR(sock->localAddrStr), NULLSTR(sock->remoteAddrStr));
diff --git a/src/util/command.c b/src/util/command.c
index dc3cfc5..1dc0090 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -2103,7 +2103,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
if (cmd->pid != -1) {
virCommandError(VIR_ERR_INTERNAL_ERROR,
- _("command is already running as pid %d"),
+ _("command is already running as pid %" PID_FORMAT),
cmd->pid);
return -1;
}
@@ -2178,7 +2178,7 @@ virPidWait(pid_t pid, int *exitstatus)
int status;
if (pid <= 0) {
- virReportSystemError(EINVAL, _("unable to wait for process %d"), pid);
+ virReportSystemError(EINVAL, _("unable to wait for process %"
PID_FORMAT), pid);
return -1;
}
@@ -2187,7 +2187,7 @@ virPidWait(pid_t pid, int *exitstatus)
errno == EINTR);
if (ret == -1) {
- virReportSystemError(errno, _("unable to wait for process %d"), pid);
+ virReportSystemError(errno, _("unable to wait for process %"
PID_FORMAT), pid);
return -1;
}
@@ -2195,7 +2195,7 @@ virPidWait(pid_t pid, int *exitstatus)
if (status != 0) {
char *st = virCommandTranslateStatus(status);
virCommandError(VIR_ERR_INTERNAL_ERROR,
- _("Child process (%d) status unexpected: %s"),
+ _("Child process (%" PID_FORMAT ") status
unexpected: %s"),
pid, NULLSTR(st));
VIR_FREE(st);
return -1;
@@ -2351,7 +2351,7 @@ void
virPidAbort(pid_t pid)
{
/* Not yet ported to mingw. Any volunteers? */
- VIR_DEBUG("failed to reap child %d, abandoning it", pid);
+ VIR_DEBUG("failed to reap child %" PID_FORMAT ", abandoning it",
pid);
}
void
diff --git a/src/util/util.h b/src/util/util.h
index 94d9282..01c29ea 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -31,6 +31,7 @@
# include <sys/select.h>
# include <sys/types.h>
# include <stdarg.h>
+# include <inttypes.h>
# ifndef MIN
# define MIN(a, b) ((a) < (b) ? (a) : (b))
@@ -39,6 +40,13 @@
# define MAX(a, b) ((a) > (b) ? (a) : (b))
# endif
+#ifdef _WIN64
+/* XXX gnu_printf prefers lld while non-gnu printf expect PRId64... */
+# define PID_FORMAT "lld"
+#else
+# define PID_FORMAT "d"
+#endif
+
ssize_t saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
ssize_t safewrite(int fd, const void *buf, size_t count)
ATTRIBUTE_RETURN_CHECK;
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index 1fd6318..f2532d4 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -69,7 +69,7 @@ int virPidFileWritePath(const char *pidfile,
goto cleanup;
}
- if (fprintf(file, "%d", pid) < 0) {
+ if (fprintf(file, "%" PID_FORMAT, pid) < 0) {
rc = -errno;
goto cleanup;
}
@@ -127,7 +127,7 @@ int virPidFileReadPath(const char *path,
goto cleanup;
}
- if (fscanf(file, "%d", pid) != 1) {
+ if (fscanf(file, "%" PID_FORMAT, pid) != 1) {
rc = -EINVAL;
VIR_FORCE_FCLOSE(file);
goto cleanup;
@@ -209,7 +209,7 @@ int virPidFileReadPathIfAlive(const char *path,
}
#endif
- if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0) {
+ if (virAsprintf(&procpath, "/proc/%" PID_FORMAT "/exe", *pid)
< 0) {
*pid = -1;
return -1;
}
--
1.7.7.5
6:06 p.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
Define PID_FORMAT and fix warnings for mingw64 x86_64 build.
Unfortunately, gnu_printf attribute check expect %lld while normal
printf is PRId64. So one warning remains.
---
src/rpc/virnetsocket.c | 4 ++--
src/util/command.c | 10 +++++-----
src/util/util.h | 8 ++++++++
src/util/virpidfile.c | 6 +++---
4 files changed, 18 insertions(+), 10 deletions(-)
This failed 'make syntax-check':
libvirt_unmarked_diagnostics
src/util/command.c-2198- PID_FORMAT ")
status unexpected: %s"),
which may be a flaw in cfg.mk rather than an actual bug, but still one
we should address.
Also, I'm not quite convinced on your approach. While it's nice to hide
the type behind a macro:
+#ifdef _WIN64
+/* XXX gnu_printf prefers lld while non-gnu printf expect PRId64... */
Libvirt should not be using non-gnu printf. What function call gave you
that warning, so that we can fix it to use gnu printf?
+# define PID_FORMAT "lld"
+#else
+# define PID_FORMAT "d"
+#endif
the decision should _not_ be based on _WIN64, but instead on a
configure-time test on the underlying type of pid_t. And since _that_
gets difficult, I'd almost rather go with the simpler approach of:
"%" PRIdMAX, (intmax_t) pid
everywhere that we currently use
"%d", pid
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:22 p.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
Hi
----- Mensaje original -----
Also, I'm not quite convinced on your approach. While it's
nice to
hide
the type behind a macro:
>
> +#ifdef _WIN64
> +/* XXX gnu_printf prefers lld while non-gnu printf expect
> PRId64... */
Libvirt should not be using non-gnu printf. What function call gave
you
that warning, so that we can fix it to use gnu printf?
They come from fprintf and fscanf here:
util/virpidfile.c: In function 'virPidFileWritePath':
util/virpidfile.c:72:5: warning: unknown conversion type character 'l' in format
[-Wformat]
util/virpidfile.c:72:5: warning: too many arguments for format [-Wformat-extra-args]
util/virpidfile.c: In function 'virPidFileReadPath':
util/virpidfile.c:130:5: warning: unknown conversion type character 'l' in format
[-Wformat]
util/virpidfile.c:130:5: warning: too many arguments for format [-Wformat-extra-args]
> +# define PID_FORMAT "lld"
> +#else
> +# define PID_FORMAT "d"
> +#endif
the decision should _not_ be based on _WIN64, but instead on a
configure-time test on the underlying type of pid_t. And since
_that_
gets difficult, I'd almost rather go with the simpler approach of:
"%" PRIdMAX, (intmax_t) pid
everywhere that we currently use
"%d", pid
I thought about using that solution, but I prefer the format macro. Tbh, I wish some of
these would be part of gnulib (perhaps some already are). glib also has a bunch of these,
although not pid_t.
6:54 p.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
On 01/25/2012 05:22 PM, Marc-André Lureau wrote:
> the decision should _not_ be based on _WIN64, but instead on a
> configure-time test on the underlying type of pid_t. And since
> _that_
> gets difficult, I'd almost rather go with the simpler approach of:
>
> "%" PRIdMAX, (intmax_t) pid
>
> everywhere that we currently use
>
> "%d", pid
>
I thought about using that solution, but I prefer the format macro.
Yes, it would be nice if <inttypes.h> had a macro for id_t. But even
then, pid_t, uid_t, and gid_t are not necessarily the same width as id_t
(id_t is necessarily as large as the other three, but they don't all
have to be the same size). And it gets unwieldy fast if you start
introducing more macros for more types.
Tbh, I wish some of these would be part of gnulib (perhaps some
already are).
Sadly, no one has made a case for extended type macros in gnulib. And
mingw points out the problem - we can't use "lld" nor PRIdMAX for pid_t,
since we don't know whether the code is targetting gnu printf, windows
printf, or a mix. I really do think we're stuck with casting to (long
long) or (intmax_t) in this case :(
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:28 a.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
On Wed, Jan 25, 2012 at 05:54:19PM -0700, Eric Blake wrote:
On 01/25/2012 05:22 PM, Marc-André Lureau wrote:
>> the decision should _not_ be based on _WIN64, but instead on a
>> configure-time test on the underlying type of pid_t. And since
>> _that_
>> gets difficult, I'd almost rather go with the simpler approach of:
>>
>> "%" PRIdMAX, (intmax_t) pid
>>
>> everywhere that we currently use
>>
>> "%d", pid
>>
>
> I thought about using that solution, but I prefer the format macro.
> Tbh, I wish some of these would be part of gnulib (perhaps some
already are).
Sadly, no one has made a case for extended type macros in gnulib. And
mingw points out the problem - we can't use "lld" nor PRIdMAX for pid_t,
since we don't know whether the code is targetting gnu printf, windows
printf, or a mix. I really do think we're stuck with casting to (long
long) or (intmax_t) in this case :(
But we can use 'lld' because we made sure all libvirt code goes
via the gnulib printf replacements which guarentee %lld works
correctly.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
1:15 p.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
On Thu, Jan 26, 2012 at 11:28 AM, Daniel P. Berrange
<berrange(a)redhat.com> wrote:
On Wed, Jan 25, 2012 at 05:54:19PM -0700, Eric Blake wrote:
> On 01/25/2012 05:22 PM, Marc-André Lureau wrote:
> >> the decision should _not_ be based on _WIN64, but instead on a
> >> configure-time test on the underlying type of pid_t. And since
> >> _that_
> >> gets difficult, I'd almost rather go with the simpler approach of:
> >>
> >> "%" PRIdMAX, (intmax_t) pid
> >>
> >> everywhere that we currently use
> >>
> >> "%d", pid
> >>
> >
> > I thought about using that solution, but I prefer the format macro.
> > Tbh, I wish some of these would be part of gnulib (perhaps some already are).
>
> Sadly, no one has made a case for extended type macros in gnulib. And
> mingw points out the problem - we can't use "lld" nor PRIdMAX for
pid_t,
> since we don't know whether the code is targetting gnu printf, windows
> printf, or a mix. I really do think we're stuck with casting to (long
> long) or (intmax_t) in this case :(
But we can use 'lld' because we made sure all libvirt code goes
via the gnulib printf replacements which guarentee %lld works
correctly.
Actually, it doesn't. The replacement is provided by the module
"stdio" which isn't used (or perhaps indirectly?). But even if it was
used, it wouldn't be the POSIX version using gnuprintf format, it
would be the "system" one. If we want the former, we would need
"fprintf-posix" module, unfortunately, it seems to have incompatible
licenses:
.gnulib/gnulib-tool: *** incompatible license on modules:
fpieee LGPL
fprintf-posix LGPL
fpucw LGPL
frexp-nolibm LGPL
frexpl-nolibm LGPL
fseterr LGPL
isnand-nolibm LGPL
isnanf-nolibm LGPL
isnanl-nolibm LGPL
printf-frexp LGPL
printf-frexpl LGPL
printf-safe LGPL
signbit LGPL
However, it would only solve the fprintf() call, not the fscanf() call
which doesn't have equivalent "fscanf-posix" yet.
An alternative to avoid casting everywhere is to use a couple of
helper functions to do the job of:
fprintf(file, "%" PID_FORMAT, pid) and fscanf(file, "%" PID_FORMAT,
pid) which I guess could be replaced by virFprintfPid() and
virFscanfPid(). Agree?
--
Marc-André Lureau
1:23 p.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
On 01/30/2012 12:15 PM, Marc-André Lureau wrote:
> But we can use 'lld' because we made sure all libvirt
code goes
> via the gnulib printf replacements which guarentee %lld works
> correctly.
Actually, it doesn't. The replacement is provided by the module
"stdio" which isn't used (or perhaps indirectly?).
Rather, the replacement is provided by the snprintf, vasprintf, and
vsnprintf, modules. As long as we go through one of those three
functions, we should be getting the GNU signature of %lld, and NOT the
mingw signature of %I64d. If we fail to go through one of those three
functions (such as a raw printf), then we have to use the common subset
that is supported by both gnu and mingw.
But even if it was
used, it wouldn't be the POSIX version using gnuprintf format, it
would be the "system" one. If we want the former, we would need
"fprintf-posix" module, unfortunately, it seems to have incompatible
licenses:
No, we intentionally do NOT want to use the native format. Anywhere
that we are calling a *printf that has not gone through a gnulib module,
we have a bug in libvirt.
However, it would only solve the fprintf() call, not the fscanf() call
which doesn't have equivalent "fscanf-posix" yet.
Correct, calls to fprintf are buggy, and fscanf is a disaster that
should be using virStrToLong_* instead.
An alternative to avoid casting everywhere is to use a couple of
helper functions to do the job of:
fprintf(file, "%" PID_FORMAT, pid) and fscanf(file, "%" PID_FORMAT,
pid) which I guess could be replaced by virFprintfPid() and
virFscanfPid(). Agree?
Those helper functions are precisely what the gnulib module snprintf and
friends _already_ should be giving us. We just have to use them correctly.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:28 p.m.
New subject: [libvirt] [PATCH 1/4] Update gnulib to fix mingw64 compilation errors
---
.gnulib | 2 +-
bootstrap | 16 ++++++++++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/.gnulib b/.gnulib
index dd6b2d7..2f0ed93 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit dd6b2d751b3c6ad417f6a4c48f2adb9d27cc59d2
+Subproject commit 2f0ed93050ab6d858738d36addd8320889e4a0e0
diff --git a/bootstrap b/bootstrap
index 2a409fc..6910abf 100755
--- a/bootstrap
+++ b/bootstrap
@@ -1,6 +1,6 @@
#! /bin/sh
# Print a version string.
-scriptversion=2012-01-16.17; # UTC
+scriptversion=2012-01-21.16; # UTC
# Bootstrap this package from checked-out sources.
@@ -87,9 +87,9 @@ gnulib_files=
: ${AUTOPOINT=autopoint}
: ${AUTORECONF=autoreconf}
-# A function to be called to edit gnulib.mk right after it's created.
+# A function to be called right after gnulib-tool is run.
# Override it via your own definition in bootstrap.conf.
-gnulib_mk_hook() { :; }
+bootstrap_post_import_hook() { :; }
# A function to be called after everything else in this script.
# Override it via your own definition in bootstrap.conf.
@@ -807,6 +807,9 @@ for file in $gnulib_files; do
symlink_to_dir "$GNULIB_SRCDIR" $file || exit
done
+bootstrap_post_import_hook \
+ || { echo >&2 "$me: bootstrap_post_import_hook failed"; exit 1; }
+
# Remove any dangling symlink matching "*.m4" or "*.[ch]" in some
# gnulib-populated directories. Such .m4 files would cause aclocal to fail.
# The following requires GNU find 4.2.3 or newer. Considering the usual
@@ -819,11 +822,16 @@ find "$m4_base" "$source_base" \
-depth \( -name '*.m4' -o -name '*.[ch]' \) \
-type l -xtype l -delete > /dev/null 2>&1
-# Tell autoreconf not to invoke autopoint or libtoolize; they were run above.
+# Some systems (RHEL 5) are using ancient autotools, for which the
+# --no-recursive option had not been invented. Detect that lack and
+# omit the option when it's not supported. FIXME in 2017: remove this
+# hack when RHEL 5 autotools are updated, or when they become irrelevant.
no_recursive=
case $($AUTORECONF --help) in
*--no-recursive*) no_recursive=--no-recursive;;
esac
+
+# Tell autoreconf not to invoke autopoint or libtoolize; they were run above.
echo "running: AUTOPOINT=true LIBTOOLIZE=true " \
"$AUTORECONF --verbose --install $no_recursive -I $m4_base $ACLOCAL_FLAGS"
AUTOPOINT=true LIBTOOLIZE=true \
--
1.7.7.6
6:28 p.m.
New subject: [libvirt] [PATCH 2/4] Fix warnings about pid_t printf format on mingw64
Define PID_FORMAT and fix warnings for mingw64 x86_64 build.
---
configure.ac | 1 +
src/rpc/virnetsocket.c | 4 ++--
src/util/command.c | 10 +++++-----
src/util/util.h | 7 +++++++
src/util/virpidfile.c | 6 +++---
5 files changed, 18 insertions(+), 10 deletions(-)
diff --git a/configure.ac b/configure.ac
index 9fb7bfc..0dd4a70 100644
--- a/configure.ac
+++ b/configure.ac
@@ -128,6 +128,7 @@ fi
AC_MSG_RESULT([$have_cpuid])
AC_CHECK_SIZEOF([long])
+AC_CHECK_SIZEOF([pid_t])
dnl Availability of various common functions (non-fatal if missing),
dnl and various less common threadsafe functions
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 67d33b7..20bee4b 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -114,7 +114,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr,
virNetSocketPtr sock;
int no_slow_start = 1;
- VIR_DEBUG("localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%d",
+ VIR_DEBUG("localAddr=%p remoteAddr=%p fd=%d errfd=%d pid=%" PID_FORMAT,
localAddr, remoteAddr,
fd, errfd, pid);
@@ -174,7 +174,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr,
sock->client = isClient;
PROBE(RPC_SOCKET_NEW,
- "sock=%p refs=%d fd=%d errfd=%d pid=%d localAddr=%s, remoteAddr=%s",
+ "sock=%p refs=%d fd=%d errfd=%d pid=%" PID_FORMAT "
localAddr=%s, remoteAddr=%s",
sock, sock->refs, fd, errfd,
pid, NULLSTR(sock->localAddrStr), NULLSTR(sock->remoteAddrStr));
diff --git a/src/util/command.c b/src/util/command.c
index e3a8371..1e4c206 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -2138,7 +2138,7 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid)
if (cmd->pid != -1) {
virCommandError(VIR_ERR_INTERNAL_ERROR,
- _("command is already running as pid %d"),
+ _("command is already running as pid %" PID_FORMAT),
cmd->pid);
return -1;
}
@@ -2214,7 +2214,7 @@ virPidWait(pid_t pid, int *exitstatus)
int status;
if (pid <= 0) {
- virReportSystemError(EINVAL, _("unable to wait for process %d"), pid);
+ virReportSystemError(EINVAL, _("unable to wait for process %"
PID_FORMAT), pid);
return -1;
}
@@ -2223,7 +2223,7 @@ virPidWait(pid_t pid, int *exitstatus)
errno == EINTR);
if (ret == -1) {
- virReportSystemError(errno, _("unable to wait for process %d"), pid);
+ virReportSystemError(errno, _("unable to wait for process %"
PID_FORMAT), pid);
return -1;
}
@@ -2231,7 +2231,7 @@ virPidWait(pid_t pid, int *exitstatus)
if (status != 0) {
char *st = virCommandTranslateStatus(status);
virCommandError(VIR_ERR_INTERNAL_ERROR,
- _("Child process (%d) status unexpected: %s"),
+ _("Child process (%" PID_FORMAT ") status
unexpected: %s"),
pid, NULLSTR(st));
VIR_FREE(st);
return -1;
@@ -2387,7 +2387,7 @@ void
virPidAbort(pid_t pid)
{
/* Not yet ported to mingw. Any volunteers? */
- VIR_DEBUG("failed to reap child %d, abandoning it", pid);
+ VIR_DEBUG("failed to reap child %" PID_FORMAT ", abandoning it",
pid);
}
void
diff --git a/src/util/util.h b/src/util/util.h
index f62cb42..3e8510d 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -31,6 +31,7 @@
# include <sys/select.h>
# include <sys/types.h>
# include <stdarg.h>
+# include <inttypes.h>
# ifndef MIN
# define MIN(a, b) ((a) < (b) ? (a) : (b))
@@ -39,6 +40,12 @@
# define MAX(a, b) ((a) > (b) ? (a) : (b))
# endif
+#if SIZEOF_PID_T == 4
+# define PID_FORMAT "d"
+#elif SIZEOF_PID_T == 8
+# define PID_FORMAT "lld"
+#endif
+
ssize_t saferead(int fd, void *buf, size_t count) ATTRIBUTE_RETURN_CHECK;
ssize_t safewrite(int fd, const void *buf, size_t count)
ATTRIBUTE_RETURN_CHECK;
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index 1fd6318..f2532d4 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -69,7 +69,7 @@ int virPidFileWritePath(const char *pidfile,
goto cleanup;
}
- if (fprintf(file, "%d", pid) < 0) {
+ if (fprintf(file, "%" PID_FORMAT, pid) < 0) {
rc = -errno;
goto cleanup;
}
@@ -127,7 +127,7 @@ int virPidFileReadPath(const char *path,
goto cleanup;
}
- if (fscanf(file, "%d", pid) != 1) {
+ if (fscanf(file, "%" PID_FORMAT, pid) != 1) {
rc = -EINVAL;
VIR_FORCE_FCLOSE(file);
goto cleanup;
@@ -209,7 +209,7 @@ int virPidFileReadPathIfAlive(const char *path,
}
#endif
- if (virAsprintf(&procpath, "/proc/%d/exe", *pid) < 0) {
+ if (virAsprintf(&procpath, "/proc/%" PID_FORMAT "/exe", *pid)
< 0) {
*pid = -1;
return -1;
}
--
1.7.7.6
5:26 p.m.
New subject: [libvirt] [PATCH 2/4] Fix warnings about pid_t printf format on mingw64
On 02/01/2012 05:28 PM, Marc-André Lureau wrote:
Define PID_FORMAT and fix warnings for mingw64 x86_64 build.
NACK to PID_FORMAT. I'd rather use %lld, (long long) pid, throughout.
But I'm incorporating your patch into a respun series, look for it soon.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:28 p.m.
New subject: [libvirt] [PATCH 3/4] virpidfile: replace fopen/fwrite/fscanf with more portable version
Replace calls to fwrite() and fscanf() with more portable-friendly
version, such as snprintf() and virStrToLong().
---
src/util/virpidfile.c | 42 +++++++++++++++++++++++++-----------------
1 files changed, 25 insertions(+), 17 deletions(-)
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index f2532d4..bbcdd53 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -54,7 +54,7 @@ int virPidFileWritePath(const char *pidfile,
{
int rc;
int fd;
- FILE *file = NULL;
+ char pidstr[INT_BUFSIZE_BOUND(pid)];
if ((fd = open(pidfile,
O_WRONLY | O_CREAT | O_TRUNC,
@@ -63,21 +63,18 @@ int virPidFileWritePath(const char *pidfile,
goto cleanup;
}
- if (!(file = VIR_FDOPEN(fd, "w"))) {
- rc = -errno;
- VIR_FORCE_CLOSE(fd);
- goto cleanup;
- }
+ snprintf(pidstr, sizeof(pidstr), "%" PID_FORMAT, pid);
- if (fprintf(file, "%" PID_FORMAT, pid) < 0) {
+ if (safewrite(fd, pidstr, strlen(pidstr)) < 0) {
rc = -errno;
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
rc = 0;
cleanup:
- if (VIR_FCLOSE(file) < 0)
+ if (VIR_CLOSE(fd) < 0)
rc = -errno;
return rc;
@@ -117,30 +114,41 @@ cleanup:
int virPidFileReadPath(const char *path,
pid_t *pid)
{
- FILE *file;
+ int fd;
int rc;
+ ssize_t bytes;
+ char pidstr[INT_BUFSIZE_BOUND(*pid)];
*pid = 0;
- if (!(file = fopen(path, "r"))) {
+ if ((fd = open(path, O_RDONLY)) < 0) {
rc = -errno;
goto cleanup;
}
- if (fscanf(file, "%" PID_FORMAT, pid) != 1) {
- rc = -EINVAL;
- VIR_FORCE_FCLOSE(file);
+ bytes = saferead(fd, pidstr, sizeof(pidstr));
+ if (bytes < 0) {
+ rc = -errno;
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
+ pidstr[bytes] = '\0';
- if (VIR_FCLOSE(file) < 0) {
- rc = -errno;
+#if SIZEOF_PID_T == 8
+ if (virStrToLong_ll(pidstr, NULL, 10, pid) < 0) {
+#elif SIZEOF_PID_T == 4
+ if (virStrToLong_i(pidstr, NULL, 10, pid) < 0) {
+#endif
+ rc = -1;
goto cleanup;
}
rc = 0;
- cleanup:
+cleanup:
+ if (VIR_CLOSE(fd) < 0)
+ rc = -errno;
+
return rc;
}
@@ -367,7 +375,7 @@ int virPidFileAcquirePath(const char *path,
/* Someone else must be racing with us, so try agin */
}
- snprintf(pidstr, sizeof(pidstr), "%u", (unsigned int)pid);
+ snprintf(pidstr, sizeof(pidstr), "%" PID_FORMAT, pid);
if (safewrite(fd, pidstr, strlen(pidstr)) < 0) {
virReportSystemError(errno,
--
1.7.7.6
5:43 p.m.
New subject: [libvirt] [PATCH 3/4] virpidfile: replace fopen/fwrite/fscanf with more portable version
On 02/01/2012 05:28 PM, Marc-André Lureau wrote:
Replace calls to fwrite() and fscanf() with more portable-friendly
version, such as snprintf() and virStrToLong().
I'm tweaking this to mention why snprintf is more portable - because
we're using gnulib for snprintf but not for fprintf.
---
src/util/virpidfile.c | 42 +++++++++++++++++++++++++-----------------
1 files changed, 25 insertions(+), 17 deletions(-)
@@ -63,21 +63,18 @@ int virPidFileWritePath(const char *pidfile,
goto cleanup;
}
- if (!(file = VIR_FDOPEN(fd, "w"))) {
- rc = -errno;
- VIR_FORCE_CLOSE(fd);
- goto cleanup;
- }
+ snprintf(pidstr, sizeof(pidstr), "%" PID_FORMAT, pid);
- if (fprintf(file, "%" PID_FORMAT, pid) < 0) {
I'm using %lld, (long long) pid here.
@@ -117,30 +114,41 @@ cleanup:
int virPidFileReadPath(const char *path,
pid_t *pid)
{
- FILE *file;
+ int fd;
int rc;
+ ssize_t bytes;
+ char pidstr[INT_BUFSIZE_BOUND(*pid)];
It's easier to scan into a long long, then ensure that conversion to
pid_t doesn't truncate.
- if (VIR_FCLOSE(file) < 0) {
- rc = -errno;
+#if SIZEOF_PID_T == 8
+ if (virStrToLong_ll(pidstr, NULL, 10, pid) < 0) {
+#elif SIZEOF_PID_T == 4
+ if (virStrToLong_i(pidstr, NULL, 10, pid) < 0) {
+#endif
That is, I'm not a fan of in-method #ifdefs, if they can be avoided by
other means.
ACK; I'm pushing it slightly amended as:
diff --git c/src/util/virpidfile.c w/src/util/virpidfile.c
index 1fd6318..34d1250 100644
--- c/src/util/virpidfile.c
+++ w/src/util/virpidfile.c
@@ -1,7 +1,7 @@
/*
* virpidfile.c: manipulation of pidfiles
*
- * Copyright (C) 2010-2011 Red Hat, Inc.
+ * Copyright (C) 2010-2012 Red Hat, Inc.
* Copyright (C) 2006, 2007 Binary Karma
* Copyright (C) 2006 Shuveb Hussain
*
@@ -54,7 +54,7 @@ int virPidFileWritePath(const char *pidfile,
{
int rc;
int fd;
- FILE *file = NULL;
+ char pidstr[INT_BUFSIZE_BOUND(pid)];
if ((fd = open(pidfile,
O_WRONLY | O_CREAT | O_TRUNC,
@@ -63,21 +63,18 @@ int virPidFileWritePath(const char *pidfile,
goto cleanup;
}
- if (!(file = VIR_FDOPEN(fd, "w"))) {
- rc = -errno;
- VIR_FORCE_CLOSE(fd);
- goto cleanup;
- }
+ snprintf(pidstr, sizeof(pidstr), "%lld", (long long) pid);
- if (fprintf(file, "%d", pid) < 0) {
+ if (safewrite(fd, pidstr, strlen(pidstr)) < 0) {
rc = -errno;
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
rc = 0;
cleanup:
- if (VIR_FCLOSE(file) < 0)
+ if (VIR_CLOSE(fd) < 0)
rc = -errno;
return rc;
@@ -117,30 +114,40 @@ cleanup:
int virPidFileReadPath(const char *path,
pid_t *pid)
{
- FILE *file;
+ int fd;
int rc;
+ ssize_t bytes;
+ long long pid_value = 0;
+ char pidstr[INT_BUFSIZE_BOUND(pid_value)];
*pid = 0;
- if (!(file = fopen(path, "r"))) {
+ if ((fd = open(path, O_RDONLY)) < 0) {
rc = -errno;
goto cleanup;
}
- if (fscanf(file, "%d", pid) != 1) {
- rc = -EINVAL;
- VIR_FORCE_FCLOSE(file);
+ bytes = saferead(fd, pidstr, sizeof(pidstr));
+ if (bytes < 0) {
+ rc = -errno;
+ VIR_FORCE_CLOSE(fd);
goto cleanup;
}
+ pidstr[bytes] = '\0';
- if (VIR_FCLOSE(file) < 0) {
- rc = -errno;
+ if (virStrToLong_ll(pidstr, NULL, 10, &pid_value) < 0 ||
+ (pid_t) pid_value != pid_value) {
+ rc = -1;
goto cleanup;
}
+ *pid = pid_value;
rc = 0;
- cleanup:
+cleanup:
+ if (VIR_CLOSE(fd) < 0)
+ rc = -errno;
+
return rc;
}
@@ -367,7 +374,7 @@ int virPidFileAcquirePath(const char *path,
/* Someone else must be racing with us, so try agin */
}
- snprintf(pidstr, sizeof(pidstr), "%u", (unsigned int)pid);
+ snprintf(pidstr, sizeof(pidstr), "%lld", (long long) pid);
if (safewrite(fd, pidstr, strlen(pidstr)) < 0) {
virReportSystemError(errno,
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:28 p.m.
New subject: [libvirt] [PATCH 4/4] Remove compile-time pid_t size check
It seems all the code use pid_t type properly, so it is safe
to drop that check now.
---
src/util/command.c | 3 ---
src/util/virpidfile.c | 2 --
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c
index 1e4c206..ce992a6 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -50,9 +50,6 @@
virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
-/* We have quite a bit of changes to make if this doesn't hold. */
-verify(sizeof(pid_t) <= sizeof(int));
-
/* Flags for virExecWithHook */
enum {
VIR_EXEC_NONE = 0,
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index bbcdd53..1fa2de9 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -311,8 +311,6 @@ cleanup:
}
-verify(sizeof(pid_t) <= sizeof(unsigned int));
-
int virPidFileAcquirePath(const char *path,
pid_t pid)
{
--
1.7.7.6
5:46 p.m.
New subject: [libvirt] [PATCH 4/4] Remove compile-time pid_t size check
On 02/01/2012 05:28 PM, Marc-André Lureau wrote:
It seems all the code use pid_t type properly, so it is safe
to drop that check now.
---
src/util/command.c | 3 ---
src/util/virpidfile.c | 2 --
2 files changed, 0 insertions(+), 5 deletions(-)
ACK to doing this, but I'll just fold it into my respin patch series.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
6:38 p.m.
New subject: [libvirt] [PATCH 1/4] Update gnulib to fix mingw64 compilation errors
On 02/01/2012 05:28 PM, Marc-André Lureau wrote:
---
.gnulib | 2 +-
bootstrap | 16 ++++++++++++----
2 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/.gnulib b/.gnulib
index dd6b2d7..2f0ed93 160000
--- a/.gnulib
+++ b/.gnulib
@@ -1 +1 @@
-Subproject commit dd6b2d751b3c6ad417f6a4c48f2adb9d27cc59d2
+Subproject commit 2f0ed93050ab6d858738d36addd8320889e4a0e0
You beat me to this. :) There have indeed been several good upstream
fixes in gnulib worth pulling in:
* .gnulib dd6b2d7...2f0ed93 (55):
typo
autoupdate
popen: Make more robust on Windows.
Fix date of recent ChangeLog entries.
pclose: Fix typo.
doc about getlogin_r, setstate.
poll tests: Make test more robust.
sys_stat: Fix support for mingw64 and MSVC.
wcwidth: Work around bug in UTF-8 locale on OpenBSD 5.0.
quotearg: Fix test failure on MacOS X 10.5.
maint.mk: sc_prohibit_canonicalize_without_use: avoid false positive
autoupdate
test-framework-sh: Fix test failure with AIX 7.1 diff.
strtoimax: eliminate need for stdint.h, inttypes.h checks
sys_time: Override 'struct timeval' on some native Windows platforms.
accept4, fcntl, socket modules: Avoid warnings on x86_64 mingw64.
fcntl: Avoid compilation error on native Windows.
select, poll, isatty: Avoid warnings on x86_64 mingw64.
doc: clarify README-release
maint.mk: use more readable (yet functionally equivalent) quoting
stdalign: relax _Alignof and tighten _Alignas test
stdalign: Document the last change.
Fix bug# typo in previous patch.
stdalign: check that alignof and offsetof are consistent
update-copyright: accept new option: UPDATE_COPYRIGHT_USE_INTERVALS=2
build-aux/ylwrap: restore x bit
pipe2: refine doc about thread-safety
posix_spawn_file_actions_addopen: Fix 2012-01-08 commit.
pipe2, assign4: document lack of thread-safety in replacement
malloca: Avoid warnings on x86_64 mingw64.
obstack: remove __STDC__ conditionals
havelib: Modern quoting.
stdint: Improve support for Android.
doc: omit trailing empty lines from INSTALL etc.
tests: avoid spurious warnings about gl_sockets_startup
locale-fr.m4: Fix for Android.
bootstrap: fail when bootstrap_post_import_hook fails
update from texinfo
maint: enable sc_trailing_blank
maint: enable sc_prohibit_openat_without_use
maint: enable sc_prohibit_cloexec_without_use
maint: enable sc_prohibit_intprops_without_use
maint: enable sc_prohibit_hash_pjw_without_use
maint: enable double-word-prohibiting rule
maint: remove empty lines at EOF, but excluding modules/*
maint: add framework to run syntax-check rules against gnulib sources
stdint: Add support for Android.
autoupdate
bootstrap: add bootstrap_post_import_hook
gitlog-to-changelog: don't use "no_"-prefixed variable name
gitlog-to-changelog: use "||", not "or" in expressions
gitlog-to-changelog: new option --no-cluster
maint: spell file systems with two words, not one
fix a typo
bootstrap: add a FIXME comment to ensure we eventually remove the hack
ACK and pushed. I'll review the rest of your series later (ran out of
time today); I suspect there's still some pid_t mis-uses to be cleaned
up first. Fortunately, beyond the gnulib update, I don't mind making
the rest of these sort of changes even after the freeze.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:26 a.m.
New subject: [libvirt] [PATCH libvirt 3/6] Fix warnings about pid_t printf format on mingw64
On Wed, Jan 25, 2012 at 09:13:23PM +0100, Marc-André Lureau wrote:
Define PID_FORMAT and fix warnings for mingw64 x86_64 build.
Unfortunately, gnu_printf attribute check expect %lld while normal
printf is PRId64. So one warning remains.
Since libvirt uses gnulib, we are guaranteed that %lld is valid
everywhere, and thus do not need any of the horrific PRId64
macros.
+#ifdef _WIN64
+/* XXX gnu_printf prefers lld while non-gnu printf expect PRId64... */
So this comment can be removed
+# define PID_FORMAT "lld"
+#else
+# define PID_FORMAT "d"
+#endif
+
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
2:13 p.m.
New subject: [libvirt] [PATCH libvirt 4/6] Remove compile-time pid_t size check
It seems all the code use pid_t type properly, so it is safe
to drop that check now.
---
src/util/command.c | 3 ---
src/util/virpidfile.c | 2 --
2 files changed, 0 insertions(+), 5 deletions(-)
diff --git a/src/util/command.c b/src/util/command.c
index 1dc0090..0518a8f 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -50,9 +50,6 @@
virReportErrorHelper(VIR_FROM_NONE, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
-/* We have quite a bit of changes to make if this doesn't hold. */
-verify(sizeof(pid_t) <= sizeof(int));
-
/* Flags for virExecWithHook */
enum {
VIR_EXEC_NONE = 0,
diff --git a/src/util/virpidfile.c b/src/util/virpidfile.c
index f2532d4..a281350 100644
--- a/src/util/virpidfile.c
+++ b/src/util/virpidfile.c
@@ -303,8 +303,6 @@ cleanup:
}
-verify(sizeof(pid_t) <= sizeof(unsigned int));
-
int virPidFileAcquirePath(const char *path,
pid_t pid)
{
--
1.7.7.5
2:13 p.m.
New subject: [libvirt] [PATCH libvirt 5/6] Cast pointer to int using intptr_t
Fix a few warnings with mingw64 x86_64.
---
src/util/logging.c | 8 ++++----
src/util/threads-win32.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/util/logging.c b/src/util/logging.c
index 8d60280..fad322f 100644
--- a/src/util/logging.c
+++ b/src/util/logging.c
@@ -329,7 +329,7 @@ static void virLogDumpAllFD(const char *msg, int len) {
for (i = 0; i < virLogNbOutputs;i++) {
if (virLogOutputs[i].f == virLogOutputToFd) {
- int fd = (long) virLogOutputs[i].data;
+ int fd = (intptr_t) virLogOutputs[i].data;
if (fd >= 0) {
ignore_value (safewrite(fd, msg, len));
@@ -791,7 +791,7 @@ static int virLogOutputToFd(const char *category ATTRIBUTE_UNUSED,
const char *str,
void *data)
{
- int fd = (long) data;
+ int fd = (intptr_t) data;
int ret;
char *msg;
@@ -808,7 +808,7 @@ static int virLogOutputToFd(const char *category ATTRIBUTE_UNUSED,
}
static void virLogCloseFd(void *data) {
- int fd = (long) data;
+ int fd = (intptr_t) data;
VIR_FORCE_CLOSE(fd);
}
@@ -826,7 +826,7 @@ static int virLogAddOutputToFile(int priority, const char *file) {
fd = open(file, O_CREAT | O_APPEND | O_WRONLY, S_IRUSR | S_IWUSR);
if (fd < 0)
return -1;
- if (virLogDefineOutput(virLogOutputToFd, virLogCloseFd, (void *)(long)fd,
+ if (virLogDefineOutput(virLogOutputToFd, virLogCloseFd, (void *)(intptr_t)fd,
priority, VIR_LOG_TO_FILE, file, 0) < 0) {
VIR_FORCE_CLOSE(fd);
return -1;
diff --git a/src/util/threads-win32.c b/src/util/threads-win32.c
index 1c33826..157439c 100644
--- a/src/util/threads-win32.c
+++ b/src/util/threads-win32.c
@@ -336,7 +336,7 @@ int virThreadSelfID(void)
/* For debugging use only; see comments in threads-pthread.c. */
int virThreadID(virThreadPtr thread)
{
- return (int)thread->thread;
+ return (intptr_t)thread->thread;
}
--
1.7.7.5
7:02 p.m.
New subject: [libvirt] [PATCH libvirt 5/6] Cast pointer to int using intptr_t
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
Fix a few warnings with mingw64 x86_64.
---
src/util/logging.c | 8 ++++----
src/util/threads-win32.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
ACK and pushed.
What a shame that (intptr_t) is the only portable type for converting
between void* and integers; but it's the right thing to do.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
2:13 p.m.
New subject: [libvirt] [PATCH libvirt 6/6] Cast timeval.tv_sec long to localtime expected type time_t
---
tools/virsh.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 999941c..246e638 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3703,7 +3703,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char *mime)
/* add mime type here */
gettimeofday(&cur_time, NULL);
- localtime_r(&cur_time.tv_sec, &time_info);
+ localtime_r((time_t *)&cur_time.tv_sec, &time_info);
strftime(timestr, sizeof(timestr), "%Y-%m-%d-%H:%M:%S", &time_info);
if (virAsprintf(&ret, "%s-%s%s", virDomainGetName(dom),
@@ -18101,7 +18101,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char
*msg_format,
* [YYYY.MM.DD HH:MM:SS SIGNATURE PID] LOG_LEVEL message
*/
gettimeofday(&stTimeval, NULL);
- stTm = localtime(&stTimeval.tv_sec);
+ stTm = localtime((time_t *)&stTimeval.tv_sec);
virBufferAsprintf(&buf, "[%d.%02d.%02d %02d:%02d:%02d %s %d] ",
(1900 + stTm->tm_year),
(1 + stTm->tm_mon),
--
1.7.7.5
7:05 p.m.
New subject: [libvirt] [PATCH libvirt 6/6] Cast timeval.tv_sec long to localtime expected type time_t
[adding bug-gnulib]
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
---
tools/virsh.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 999941c..246e638 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -3703,7 +3703,7 @@ vshGenFileName(vshControl *ctl, virDomainPtr dom, const char
*mime)
/* add mime type here */
gettimeofday(&cur_time, NULL);
- localtime_r(&cur_time.tv_sec, &time_info);
+ localtime_r((time_t *)&cur_time.tv_sec, &time_info);
NAK. tv_sec should already be time_t. This is a bug in your system
header definition of 'struct timespec', or in something that gnulib has
done to that struct in the meantime; and we should make gnulib work
around it if we can't get if fixed in mingw.
Can you help us identify why we are getting a type mismatch warning from
the compiler in the first place?
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
3:53 p.m.
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
Fixes the following warning:
util/virterror.c:1242:31: warning: declaration of 'errcode' shadows a global
declaration [-Wshadow]
Which header is polluting the namespace with a non-standard name?
---
src/util/virterror.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
At any rate, ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4:17 p.m.
----- Mensaje original -----
On 01/25/2012 01:13 PM, Marc-André Lureau wrote:
> Fixes the following warning:
> util/virterror.c:1242:31: warning: declaration of 'errcode' shadows
> a global declaration [-Wshadow]
Which header is polluting the namespace with a non-standard name?
crtdefs.h
10:typedef int errcode;
4670
days inactive
4686
days old
30 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Marc-André Lureau -
Marc-André Lureau