Re: [Xen-devel] [PATCH v5 22/24] libxlu: introduce new APIs

2015-02-16 Thread Wei Liu
On Fri, Feb 13, 2015 at 02:12:29PM +, Ian Jackson wrote:
 Wei Liu writes ([PATCH v5 22/24] libxlu: introduce new APIs):
  These APIs can be used to manipulate XLU_ConfigValue and XLU_ConfigList.
  
  +if (value-type != XLU_STRING) {
  +if (!dont_warn)
  +fprintf(cfg-report, warning: value is not a string\n);
  +*value_r = NULL;
  +return EINVAL;
 
 This message needs to include the file and line number, or it is very
 hard for the user to use.  The other call sites (which are based on
 `find') require the caller to provide a name, which means that the
 setting name can be printed too.  Maybe you could do something
 similar.
 

It is a bit different from a setting because a value doesn't have a
name.

I've added another patch to record line and column number for a value so
that we can use them here.

 If you were feeling keen you could replace these formulaic things with
 something like:
return report_bad_cfg(dont_warn, cfg, set, n, value is not a string);
 or
return REPORT_BAD_CFG(value is not a string);
 (being a function or macro which always returns EINVAL), or some such.
 

Do feel very keen about this since the format string differs from
functions. And it's only one printf anyway.

Wei.

 Thanks,
 Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v5 22/24] libxlu: introduce new APIs

2015-02-16 Thread Wei Liu
On Mon, Feb 16, 2015 at 07:10:46PM +, Wei Liu wrote:
[...]
  (being a function or macro which always returns EINVAL), or some such.
  
 
 Do feel very keen about this since the format string differs from

Don't...

 functions. And it's only one printf anyway.
 
 Wei.
 
  Thanks,
  Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


Re: [Xen-devel] [PATCH v5 22/24] libxlu: introduce new APIs

2015-02-13 Thread Ian Jackson
Wei Liu writes ([PATCH v5 22/24] libxlu: introduce new APIs):
 These APIs can be used to manipulate XLU_ConfigValue and XLU_ConfigList.
 
 +if (value-type != XLU_STRING) {
 +if (!dont_warn)
 +fprintf(cfg-report, warning: value is not a string\n);
 +*value_r = NULL;
 +return EINVAL;

This message needs to include the file and line number, or it is very
hard for the user to use.  The other call sites (which are based on
`find') require the caller to provide a name, which means that the
setting name can be printed too.  Maybe you could do something
similar.

If you were feeling keen you could replace these formulaic things with
something like:
   return report_bad_cfg(dont_warn, cfg, set, n, value is not a string);
or
   return REPORT_BAD_CFG(value is not a string);
(being a function or macro which always returns EINVAL), or some such.

Thanks,
Ian.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH v5 22/24] libxlu: introduce new APIs

2015-02-12 Thread Wei Liu
These APIs can be used to manipulate XLU_ConfigValue and XLU_ConfigList.

APIs introduced:
1. xlu_cfg_value_type
2. xlu_cfg_value_get_string
3. xlu_cfg_value_get_list
4. xlu_cfg_get_listitem2

Move some definitions from private header to public header as needed.

Signed-off-by: Wei Liu wei.l...@citrix.com
Cc: Ian Jackson ian.jack...@eu.citrix.com
Cc: Ian Campbell ian.campb...@citrix.com
---
Changes in v5:
1. Use calling convention like old APIs.
---
 tools/libxl/libxlu_cfg.c  | 41 +
 tools/libxl/libxlu_internal.h |  7 ---
 tools/libxl/libxlutil.h   | 13 +
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 611f5ec..46b1d4f 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -199,6 +199,47 @@ static int find_atom(const XLU_Config *cfg, const char *n,
 return 0;
 }
 
+
+enum XLU_ConfigValueType xlu_cfg_value_type(const XLU_ConfigValue *value)
+{
+return value-type;
+}
+
+int xlu_cfg_value_get_string(const XLU_Config *cfg, XLU_ConfigValue *value,
+ char **value_r, int dont_warn)
+{
+if (value-type != XLU_STRING) {
+if (!dont_warn)
+fprintf(cfg-report, warning: value is not a string\n);
+*value_r = NULL;
+return EINVAL;
+}
+
+*value_r = value-u.string;
+return 0;
+}
+
+int xlu_cfg_value_get_list(const XLU_Config *cfg, XLU_ConfigValue *value,
+   XLU_ConfigList **value_r, int dont_warn)
+{
+if (value-type != XLU_LIST) {
+if (!dont_warn)
+fprintf(cfg-report, warning: value is not a list\n);
+*value_r = NULL;
+return EINVAL;
+}
+
+*value_r = value-u.list;
+return 0;
+}
+
+XLU_ConfigValue *xlu_cfg_get_listitem2(const XLU_ConfigList *list,
+   int entry)
+{
+if (entry  0 || entry = list-nvalues) return NULL;
+return list-values[entry];
+}
+
 int xlu_cfg_get_string(const XLU_Config *cfg, const char *n,
const char **value_r, int dont_warn) {
 XLU_ConfigSetting *set;
diff --git a/tools/libxl/libxlu_internal.h b/tools/libxl/libxlu_internal.h
index 092a17a..24ed6d4 100644
--- a/tools/libxl/libxlu_internal.h
+++ b/tools/libxl/libxlu_internal.h
@@ -25,13 +25,6 @@
 
 #include libxlutil.h
 
-enum XLU_ConfigValueType {
-XLU_STRING,
-XLU_LIST,
-};
-
-typedef struct XLU_ConfigValue XLU_ConfigValue;
-
 typedef struct XLU_ConfigList {
 int avalues; /* available slots */
 int nvalues; /* actual occupied slots */
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
index 0333e55..989605a 100644
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -20,9 +20,15 @@
 
 #include libxl.h
 
+enum XLU_ConfigValueType {
+XLU_STRING,
+XLU_LIST,
+};
+
 /* Unless otherwise stated, all functions return an errno value. */
 typedef struct XLU_Config XLU_Config;
 typedef struct XLU_ConfigList XLU_ConfigList;
+typedef struct XLU_ConfigValue XLU_ConfigValue;
 
 XLU_Config *xlu_cfg_init(FILE *report, const char *report_filename);
   /* 0 means we got ENOMEM. */
@@ -66,6 +72,13 @@ const char *xlu_cfg_get_listitem(const XLU_ConfigList*, int 
entry);
   /* xlu_cfg_get_listitem cannot fail, except that if entry is
* out of range it returns 0 (not setting errno) */
 
+enum XLU_ConfigValueType xlu_cfg_value_type(const XLU_ConfigValue *value);
+int xlu_cfg_value_get_string(const XLU_Config *cfg,  XLU_ConfigValue *value,
+ char **value_r, int dont_warn);
+int xlu_cfg_value_get_list(const XLU_Config *cfg, XLU_ConfigValue *value,
+   XLU_ConfigList **value_r, int dont_warn);
+XLU_ConfigValue *xlu_cfg_get_listitem2(const XLU_ConfigList *list,
+   int entry);
 
 /*
  * Disk specification parsing.
-- 
1.9.1


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel