On Thu, 2008-02-07 at 05:02 -0500, Daniel Veillard wrote:
> On Wed, Feb 06, 2008 at 11:07:55PM +0000, Mark McLoughlin wrote:
> > Alternative is to not build with -Winline.
>
> That sounds a weak way to try to avoid a problem, we should
> not rely on just compiler options to get the code to compile and link.
>
> My preference would be to use the patch to make them real internal
> APIs without exporting all the functions, I think only xstrtol_i is
> used by external programs (virsh and qemud), and maybe we can add only
> that one to the list of exported symbols.
Yeah, good point. Like this?
Cheers,
Mark.
Subject: Do not inline xstrtol functions
From: Mark McLoughlin <markmc@xxxxxxxxxx>
Our strtol() variants are all marked "static inline"
and with gcc 4.3 we get:
internal.h:272: error: inlining failed in call to 'xstrtol_i': call is unlikely and code size would grow
This patch renames them to virStrToLong() and exports
them from the library as private symbols.
Alternative is to not build with -Winline.
Signed-off-by: Mark McLoughlin <markmc@xxxxxxxxxx>
Index: libvirt/src/internal.h
===================================================================
--- libvirt.orig/src/internal.h 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/internal.h 2008-02-07 10:05:46.000000000 +0000
@@ -261,85 +261,6 @@
int __virDomainMigratePerform (virDomainPtr domain, const char *cookie, int cookielen, const char *uri, unsigned long flags, const char *dname, unsigned long bandwidth);
virDomainPtr __virDomainMigrateFinish (virConnectPtr dconn, const char *dname, const char *cookie, int cookielen, const char *uri, unsigned long flags);
-/* Like strtol, but produce an "int" result, and check more carefully.
- Return 0 upon success; return -1 to indicate failure.
- When END_PTR is NULL, the byte after the final valid digit must be NUL.
- Otherwise, it's like strtol and lets the caller check any suffix for
- validity. This function is careful to return -1 when the string S
- represents a number that is not representable as an "int". */
-static inline int
-xstrtol_i(char const *s, char **end_ptr, int base, int *result)
-{
- long int val;
- char *p;
- int err;
-
- errno = 0;
- val = strtol(s, &p, base);
- err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
- if (end_ptr)
- *end_ptr = p;
- if (err)
- return -1;
- *result = val;
- return 0;
-}
-
-/* Just like xstrtol_i, above, but produce an "unsigned int" value. */
-static inline int
-xstrtol_ui(char const *s, char **end_ptr, int base, unsigned int *result)
-{
- unsigned long int val;
- char *p;
- int err;
-
- errno = 0;
- val = strtoul(s, &p, base);
- err = (errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
- if (end_ptr)
- *end_ptr = p;
- if (err)
- return -1;
- *result = val;
- return 0;
-}
-
-static inline int
-xstrtol_ll(char const *s, char **end_ptr, int base, long long *result)
-{
- long long val;
- char *p;
- int err;
-
- errno = 0;
- val = strtoll(s, &p, base);
- err = (errno || (!end_ptr && *p) || p == s || (long long) val != val);
- if (end_ptr)
- *end_ptr = p;
- if (err)
- return -1;
- *result = val;
- return 0;
-}
-
-/* Just like xstrtol_i, above, but produce an "unsigned long long" value. */
-static inline int
-xstrtol_ull(char const *s, char **end_ptr, int base, unsigned long long *result)
-{
- unsigned long long val;
- char *p;
- int err;
-
- errno = 0;
- val = strtoull(s, &p, base);
- err = (errno || (!end_ptr && *p) || p == s || (unsigned long long) val != val);
- if (end_ptr)
- *end_ptr = p;
- if (err)
- return -1;
- *result = val;
- return 0;
-}
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: libvirt/src/util.c
===================================================================
--- libvirt.orig/src/util.c 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/util.c 2008-02-07 10:06:22.000000000 +0000
@@ -549,6 +549,87 @@
return 0;
}
+/* Like strtol, but produce an "int" result, and check more carefully.
+ Return 0 upon success; return -1 to indicate failure.
+ When END_PTR is NULL, the byte after the final valid digit must be NUL.
+ Otherwise, it's like strtol and lets the caller check any suffix for
+ validity. This function is careful to return -1 when the string S
+ represents a number that is not representable as an "int". */
+int
+__virStrToLong_i(char const *s, char **end_ptr, int base, int *result)
+{
+ long int val;
+ char *p;
+ int err;
+
+ errno = 0;
+ val = strtol(s, &p, base);
+ err = (errno || (!end_ptr && *p) || p == s || (int) val != val);
+ if (end_ptr)
+ *end_ptr = p;
+ if (err)
+ return -1;
+ *result = val;
+ return 0;
+}
+
+/* Just like virStrToLong_i, above, but produce an "unsigned int" value. */
+int
+virStrToLong_ui(char const *s, char **end_ptr, int base, unsigned int *result)
+{
+ unsigned long int val;
+ char *p;
+ int err;
+
+ errno = 0;
+ val = strtoul(s, &p, base);
+ err = (errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
+ if (end_ptr)
+ *end_ptr = p;
+ if (err)
+ return -1;
+ *result = val;
+ return 0;
+}
+
+/* Just like virStrToLong_i, above, but produce an "long long" value. */
+int
+virStrToLong_ll(char const *s, char **end_ptr, int base, long long *result)
+{
+ long long val;
+ char *p;
+ int err;
+
+ errno = 0;
+ val = strtoll(s, &p, base);
+ err = (errno || (!end_ptr && *p) || p == s || (long long) val != val);
+ if (end_ptr)
+ *end_ptr = p;
+ if (err)
+ return -1;
+ *result = val;
+ return 0;
+}
+
+/* Just like virStrToLong_i, above, but produce an "unsigned long long" value. */
+int
+virStrToLong_ull(char const *s, char **end_ptr, int base, unsigned long long *result)
+{
+ unsigned long long val;
+ char *p;
+ int err;
+
+ errno = 0;
+ val = strtoull(s, &p, base);
+ err = (errno || (!end_ptr && *p) || p == s || (unsigned long long) val != val);
+ if (end_ptr)
+ *end_ptr = p;
+ if (err)
+ return -1;
+ *result = val;
+ return 0;
+}
+
/*
* Local variables:
* indent-tabs-mode: nil
Index: libvirt/src/util.h
===================================================================
--- libvirt.orig/src/util.h 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/util.h 2008-02-07 10:06:11.000000000 +0000
@@ -56,4 +56,23 @@
unsigned int buflen);
+int __virStrToLong_i(char const *s,
+ char **end_ptr,
+ int base,
+ int *result);
+#define virStrToLong_i(s,e,b,r) __virStrToLong_i((s),(e),(b),(r))
+
+int virStrToLong_ui(char const *s,
+ char **end_ptr,
+ int base,
+ unsigned int *result);
+int virStrToLong_ll(char const *s,
+ char **end_ptr,
+ int base,
+ long long *result);
+int virStrToLong_ull(char const *s,
+ char **end_ptr,
+ int base,
+ unsigned long long *result);
+
#endif /* __VIR_UTIL_H__ */
Index: libvirt/src/nodeinfo.c
===================================================================
--- libvirt.orig/src/nodeinfo.c 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/nodeinfo.c 2008-02-07 10:05:46.000000000 +0000
@@ -35,6 +35,7 @@
#include "nodeinfo.h"
#include "physmem.h"
+#include "util.h"
#ifdef __linux__
#define CPUINFO_PATH "/proc/cpuinfo"
@@ -79,7 +80,7 @@
"parsing cpuinfo cpu MHz");
return -1;
}
- if (xstrtol_ui(buf+1, &p, 10, &ui) == 0
+ if (virStrToLong_ui(buf+1, &p, 10, &ui) == 0
/* Accept trailing fractional part. */
&& (*p == '\0' || *p == '.' || isspace(*p)))
nodeinfo->mhz = ui;
@@ -95,7 +96,7 @@
"parsing cpuinfo cpu cores %c", *buf);
return -1;
}
- if (xstrtol_ui(buf+1, &p, 10, &id) == 0
+ if (virStrToLong_ui(buf+1, &p, 10, &id) == 0
&& (*p == '\0' || isspace(*p))
&& id > nodeinfo->cores)
nodeinfo->cores = id;
Index: libvirt/src/stats_linux.c
===================================================================
--- libvirt.orig/src/stats_linux.c 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/stats_linux.c 2008-02-07 10:05:46.000000000 +0000
@@ -24,7 +24,7 @@
#include <xs.h>
#endif
-#include "internal.h"
+#include "util.h"
#include "xen_unified.h"
#include "stats_linux.h"
@@ -263,7 +263,7 @@
if (path[4] != '\0') {
if (!isdigit(path[4]) || path[4] == '0' ||
- xstrtol_i(path+4, NULL, 10, &part) < 0 ||
+ virStrToLong_i(path+4, NULL, 10, &part) < 0 ||
part < 1 || part > 15) {
statsErrorFunc (conn, VIR_ERR_INVALID_ARG, __FUNCTION__,
"invalid path, partition numbers for xvdN must be in range 1 - 15",
@@ -307,7 +307,7 @@
p = path + 3;
}
if (p && (!isdigit(*p) || *p == '0' ||
- xstrtol_i(p, NULL, 10, &part) < 0 ||
+ virStrToLong_i(p, NULL, 10, &part) < 0 ||
part < 1 || part > 15)) {
statsErrorFunc (conn, VIR_ERR_INVALID_ARG, __FUNCTION__,
"invalid path, partition numbers for sdN must be in range 1 - 15",
@@ -333,7 +333,7 @@
if (path[3] != '\0') {
if (!isdigit(path[3]) || path[3] == '0' ||
- xstrtol_i(path+3, NULL, 10, &part) < 0 ||
+ virStrToLong_i(path+3, NULL, 10, &part) < 0 ||
part < 1 || part > 63) {
statsErrorFunc (conn, VIR_ERR_INVALID_ARG, __FUNCTION__,
"invalid path, partition numbers for hdN must be in range 1 - 63",
Index: libvirt/src/virsh.c
===================================================================
--- libvirt.orig/src/virsh.c 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/virsh.c 2008-02-07 10:05:46.000000000 +0000
@@ -46,7 +46,6 @@
#include <readline/history.h>
#endif
-#include "internal.h"
#include "console.h"
#include "util.h"
@@ -2914,7 +2913,7 @@
(obj->stringval == NULL) || (obj->stringval[0] == 0)) {
goto cleanup;
}
- if (xstrtol_i((const char *)obj->stringval, NULL, 10, &port) || port < 0)
+ if (virStrToLong_i((const char *)obj->stringval, NULL, 10, &port) || port < 0)
goto cleanup;
xmlXPathFreeObject(obj);
@@ -3959,7 +3958,7 @@
/* try it by ID */
if (flag & VSH_BYID) {
- if (xstrtol_i(n, NULL, 10, &id) == 0 && id >= 0) {
+ if (virStrToLong_i(n, NULL, 10, &id) == 0 && id >= 0) {
vshDebug(ctl, 5, "%s: <%s> seems like domain ID\n",
cmd->def->name, optname);
dom = virDomainLookupByID(ctl->conn, id);
Index: libvirt/src/xend_internal.c
===================================================================
--- libvirt.orig/src/xend_internal.c 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/xend_internal.c 2008-02-07 10:05:46.000000000 +0000
@@ -35,7 +35,7 @@
#include "libvirt/libvirt.h"
#include "driver.h"
-#include "internal.h"
+#include "util.h"
#include "sexpr.h"
#include "xml.h"
#include "buf.h"
@@ -3049,7 +3049,7 @@
(t->u.s.car->u.s.cdr->kind == SEXPR_CONS)) {
for (t = t->u.s.car->u.s.cdr->u.s.car; t->kind == SEXPR_CONS; t = t->u.s.cdr)
if (t->u.s.car->kind == SEXPR_VALUE
- && xstrtol_i(t->u.s.car->u.value, NULL, 10, &cpu) == 0
+ && virStrToLong_i(t->u.s.car->u.value, NULL, 10, &cpu) == 0
&& cpu >= 0
&& (VIR_CPU_MAPLEN(cpu+1) <= maplen)) {
VIR_USE_CPU(cpumap, cpu);
Index: libvirt/qemud/qemud.c
===================================================================
--- libvirt.orig/qemud/qemud.c 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/qemud/qemud.c 2008-02-07 10:05:46.000000000 +0000
@@ -52,7 +52,7 @@
#include "internal.h"
#include "getaddrinfo.h"
-#include "../src/internal.h"
+#include "../src/util.h"
#include "../src/remote_internal.h"
#include "../src/conf.h"
#include "event.h"
@@ -1889,7 +1889,7 @@
GET_CONF_STR (conf, filename, unix_sock_ro_perms);
if (unix_sock_ro_perms) {
- if (xstrtol_i (unix_sock_ro_perms, NULL, 8, &unix_sock_ro_mask) != 0) {
+ if (virStrToLong_i (unix_sock_ro_perms, NULL, 8, &unix_sock_ro_mask) != 0) {
qemudLog (QEMUD_ERR, "Failed to parse mode '%s'",
unix_sock_ro_perms);
goto free_and_fail;
@@ -1900,7 +1900,7 @@
GET_CONF_STR (conf, filename, unix_sock_rw_perms);
if (unix_sock_rw_perms) {
- if (xstrtol_i (unix_sock_rw_perms, NULL, 8, &unix_sock_rw_mask) != 0) {
+ if (virStrToLong_i (unix_sock_rw_perms, NULL, 8, &unix_sock_rw_mask) != 0) {
qemudLog (QEMUD_ERR, "Failed to parse mode '%s'",
unix_sock_rw_perms);
goto free_and_fail;
@@ -2045,7 +2045,7 @@
break;
case 't':
- if (xstrtol_i(optarg, &tmp, 10, &timeout) != 0
+ if (virStrToLong_i(optarg, &tmp, 10, &timeout) != 0
|| timeout <= 0
/* Ensure that we can multiply by 1000 without overflowing. */
|| timeout > INT_MAX / 1000)
Index: libvirt/src/libvirt_sym.version
===================================================================
--- libvirt.orig/src/libvirt_sym.version 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/libvirt_sym.version 2008-02-07 10:06:55.000000000 +0000
@@ -132,6 +132,7 @@
__virDomainMigrateFinish;
__virFileReadAll;
+ __virStrToLong_i;
local: *;
};
Index: libvirt/src/nodeinfo.h
===================================================================
--- libvirt.orig/src/nodeinfo.h 2008-02-07 10:05:32.000000000 +0000
+++ libvirt/src/nodeinfo.h 2008-02-07 10:05:46.000000000 +0000
@@ -24,7 +24,7 @@
#ifndef __VIR_NODEINFO_H__
#define __VIR_NODEINFO_H__
-#include "internal.h"
+#include "libvirt/libvirt.h"
#ifdef __cplusplus
extern "C" {
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list