[libvirt] [PATCH] Include stdint.h in libvirt.h for INT64_MAX
I found this one while trying to compile ruby-libvirt. Matthias
On Wed, Mar 09, 2011 at 10:49:34AM +0100, Matthias Bolte wrote:
I found this one while trying to compile ruby-libvirt.
Matthias
From 6858713a7ea7fad961acbc4d4f3c0c53ede2302d Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Wed, 9 Mar 2011 10:42:49 +0100 Subject: [PATCH] Include stdint.h in libvirt.h for INT64_MAX
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED uses INT64_MAX but stdint.h was not included. Therefore, libvirt.h was not selfcontained.
I'm a little wary of doing this, because stdint.h isn't neccessary the most portable header file & while libvirt internally can rely on gnulib to fix problems, apps building against libvirt don't have that. I think it could be better to just define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED directly to the right value, rather than in terms of INT64_MAX 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 :|
On Wed, Mar 09, 2011 at 12:18:44PM +0000, Daniel P. Berrange wrote:
On Wed, Mar 09, 2011 at 10:49:34AM +0100, Matthias Bolte wrote:
I found this one while trying to compile ruby-libvirt.
Matthias
From 6858713a7ea7fad961acbc4d4f3c0c53ede2302d Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Wed, 9 Mar 2011 10:42:49 +0100 Subject: [PATCH] Include stdint.h in libvirt.h for INT64_MAX
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED uses INT64_MAX but stdint.h was not included. Therefore, libvirt.h was not selfcontained.
I'm a little wary of doing this, because stdint.h isn't neccessary the most portable header file & while libvirt internally can rely on gnulib to fix problems, apps building against libvirt don't have that.
I think it could be better to just define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED directly to the right value, rather than in terms of INT64_MAX
Agreed we should try to limit the dependancies from public headers to the minimum, in that case I'm fine hardcoding the value. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
2011/3/9 Daniel Veillard <veillard@redhat.com>:
On Wed, Mar 09, 2011 at 12:18:44PM +0000, Daniel P. Berrange wrote:
On Wed, Mar 09, 2011 at 10:49:34AM +0100, Matthias Bolte wrote:
I found this one while trying to compile ruby-libvirt.
Matthias
From 6858713a7ea7fad961acbc4d4f3c0c53ede2302d Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Wed, 9 Mar 2011 10:42:49 +0100 Subject: [PATCH] Include stdint.h in libvirt.h for INT64_MAX
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED uses INT64_MAX but stdint.h was not included. Therefore, libvirt.h was not selfcontained.
I'm a little wary of doing this, because stdint.h isn't neccessary the most portable header file & while libvirt internally can rely on gnulib to fix problems, apps building against libvirt don't have that.
I think it could be better to just define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED directly to the right value, rather than in terms of INT64_MAX
Agreed we should try to limit the dependancies from public headers to the minimum, in that case I'm fine hardcoding the value.
Daniel
Okay, so here's v2. Matthias
On Wed, Mar 09, 2011 at 02:01:06PM +0100, Matthias Bolte wrote:
2011/3/9 Daniel Veillard <veillard@redhat.com>:
On Wed, Mar 09, 2011 at 12:18:44PM +0000, Daniel P. Berrange wrote:
On Wed, Mar 09, 2011 at 10:49:34AM +0100, Matthias Bolte wrote:
I found this one while trying to compile ruby-libvirt.
Matthias
From 6858713a7ea7fad961acbc4d4f3c0c53ede2302d Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Wed, 9 Mar 2011 10:42:49 +0100 Subject: [PATCH] Include stdint.h in libvirt.h for INT64_MAX
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED uses INT64_MAX but stdint.h was not included. Therefore, libvirt.h was not selfcontained.
I'm a little wary of doing this, because stdint.h isn't neccessary the most portable header file & while libvirt internally can rely on gnulib to fix problems, apps building against libvirt don't have that.
I think it could be better to just define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED directly to the right value, rather than in terms of INT64_MAX
Agreed we should try to limit the dependancies from public headers to the minimum, in that case I'm fine hardcoding the value.
Daniel
Okay, so here's v2.
Matthias
From f85a17d3493fc3c2c29da6460060338b4bbd73da Mon Sep 17 00:00:00 2001 From: Matthias Bolte <matthias.bolte@googlemail.com> Date: Wed, 9 Mar 2011 10:42:49 +0100 Subject: [PATCH] Don't use INT64_MAX in libvirt.h because it requires stdint.h
VIR_DOMAIN_MEMORY_PARAM_UNLIMITED uses INT64_MAX but stdint.h was not and should not be included. Therefore, libvirt.h was not self-contained.
Instead of including stdint.h specify the value directly. --- include/libvirt/libvirt.h.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 618b350..82e45d7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -705,7 +705,7 @@ typedef enum { * Macro providing the virMemoryParameter value that indicates "unlimited" */
-#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED (INT64_MAX >> 10) +#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED 9007199254740991 /* = INT64_MAX >> 10 */
Hum shouldn't that be 9007199254740991UL to provide type information ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 03/09/2011 06:47 AM, Daniel Veillard wrote:
Instead of including stdint.h specify the value directly. --- include/libvirt/libvirt.h.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 618b350..82e45d7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -705,7 +705,7 @@ typedef enum { * Macro providing the virMemoryParameter value that indicates "unlimited" */
-#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED (INT64_MAX >> 10) +#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED 9007199254740991 /* = INT64_MAX >> 10 */
Hum shouldn't that be 9007199254740991UL to provide type information ?
UL still won't work on 32-bit platforms. You need the LL (or ULL) suffix. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/3/9 Eric Blake <eblake@redhat.com>:
On 03/09/2011 06:47 AM, Daniel Veillard wrote:
Instead of including stdint.h specify the value directly. --- include/libvirt/libvirt.h.in | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 618b350..82e45d7 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -705,7 +705,7 @@ typedef enum { * Macro providing the virMemoryParameter value that indicates "unlimited" */
-#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED (INT64_MAX >> 10) +#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED 9007199254740991 /* = INT64_MAX >> 10 */
Hum shouldn't that be 9007199254740991UL to provide type information ?
UL still won't work on 32-bit platforms. You need the LL (or ULL) suffix.
And now the final v3 for something that should have been a trivial patch :) Matthias
On 03/11/2011 02:12 PM, Matthias Bolte wrote:
UL still won't work on 32-bit platforms. You need the LL (or ULL) suffix.
And now the final v3 for something that should have been a trivial patch :)
application/mbox? Why not text/plain for the MIME type?
+++ b/include/libvirt/libvirt.h.in @@ -772,7 +772,7 @@ typedef enum { * Macro providing the virMemoryParameter value that indicates "unlimited" */
-#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED (INT64_MAX >> 10) +#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED 9007199254740991LL /* = INT64_MAX >> 10 */
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
2011/3/11 Eric Blake <eblake@redhat.com>:
On 03/11/2011 02:12 PM, Matthias Bolte wrote:
UL still won't work on 32-bit platforms. You need the LL (or ULL) suffix.
And now the final v3 for something that should have been a trivial patch :)
application/mbox? Why not text/plain for the MIME type?
Probably because Google Mail decided to detect this one that way, typically it uses text/x-diff for attached patches. Unfortunately it doesn't let me set the MIME type.
+++ b/include/libvirt/libvirt.h.in @@ -772,7 +772,7 @@ typedef enum { * Macro providing the virMemoryParameter value that indicates "unlimited" */
-#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED (INT64_MAX >> 10) +#define VIR_DOMAIN_MEMORY_PARAM_UNLIMITED 9007199254740991LL /* = INT64_MAX >> 10 */
ACK.
Thanks, pushed it, finally. Matthias
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Eric Blake -
Matthias Bolte