[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 20/23] libxlu: record line and column number when parsing values



On Fri, Mar 06, 2015 at 04:11:53PM +0000, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 02:30:27PM +0000, Ian Jackson wrote:
> > Wei Liu writes ("Re: [PATCH v6 20/23] libxlu: record line and column number 
> > when parsing values"):
> > > Ack. This is how it looks like now. The changes are
> > > 
> > > 1. Use YYLTYPE instead of line and column.
> > 
> > This is better but why not have YYLTYPE inside XLU_ConfigValue ?
> > 
> 
> That appears to cause some kind of circular inclusion problem. I will
> see what I can do.
> 

I use a trick to work around this. That is, only have a pointer in
XLU_ConfigValue.  Then malloc XLU_ConfigValue and YYLTYPE in one go and
set the pointer accordingly. 

This way we avoid including libxlu_cfg_i.h in libxlu_internal.h and don't
need to worry about leaking memory. 

---8<---
From ce87242c706549c42581971e8e35ffcd4e6c1da7 Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Mon, 16 Feb 2015 19:01:12 +0000
Subject: [PATCH] libxlu: record location when parsing values

Originally only setting has line number recorded. Since we're moving to
more sophisticated API, record the location for individual value. It is
useful for error reporting.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
Changes in v7:
1. Use yylloc in empty rule.
1. Use YYLTYPE instead of individual line / column values.
---
 tools/libxl/libxlu_cfg.c      | 14 ++++++++++----
 tools/libxl/libxlu_cfg_i.h    |  5 +++--
 tools/libxl/libxlu_cfg_y.c    |  6 +++---
 tools/libxl/libxlu_cfg_y.y    |  6 +++---
 tools/libxl/libxlu_internal.h |  2 ++
 5 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/tools/libxl/libxlu_cfg.c b/tools/libxl/libxlu_cfg.c
index 611f5ec..858f894 100644
--- a/tools/libxl/libxlu_cfg.c
+++ b/tools/libxl/libxlu_cfg.c
@@ -311,16 +311,19 @@ const char *xlu_cfg_get_listitem(const XLU_ConfigList 
*list, int entry) {
 }
 
 
-XLU_ConfigValue *xlu__cfg_string_mk(CfgParseContext *ctx, char *atom)
+XLU_ConfigValue *xlu__cfg_string_mk(CfgParseContext *ctx, char *atom,
+                                    YYLTYPE *loc)
 {
     XLU_ConfigValue *value = NULL;
 
     if (ctx->err) goto x;
 
-    value = malloc(sizeof(*value));
+    value = malloc(sizeof(*value)+sizeof(*loc));
     if (!value) goto xe;
     value->type = XLU_STRING;
     value->u.string = atom;
+    value->loc = (YYLTYPE *)(value+1);
+    memcpy(value->loc, loc, sizeof(*loc));
 
     return value;
 
@@ -333,7 +336,8 @@ XLU_ConfigValue *xlu__cfg_string_mk(CfgParseContext *ctx, 
char *atom)
 }
 
 XLU_ConfigValue *xlu__cfg_list_mk(CfgParseContext *ctx,
-                                  XLU_ConfigValue *val)
+                                  XLU_ConfigValue *val,
+                                  YYLTYPE *loc)
 {
     XLU_ConfigValue *value = NULL;
     XLU_ConfigValue **values = NULL;
@@ -344,12 +348,14 @@ XLU_ConfigValue *xlu__cfg_list_mk(CfgParseContext *ctx,
     if (!values) goto xe;
     values[0] = val;
 
-    value = malloc(sizeof(*value));
+    value = malloc(sizeof(*value)+sizeof(*loc));
     if (!value) goto xe;
     value->type = XLU_LIST;
     value->u.list.nvalues = 1;
     value->u.list.avalues = 1;
     value->u.list.values = values;
+    value->loc = (YYLTYPE *)(value+1);
+    memcpy(value->loc, loc, sizeof(*loc));
 
     return value;
 
diff --git a/tools/libxl/libxlu_cfg_i.h b/tools/libxl/libxlu_cfg_i.h
index 11dc33f..1b59b33 100644
--- a/tools/libxl/libxlu_cfg_i.h
+++ b/tools/libxl/libxlu_cfg_i.h
@@ -26,9 +26,10 @@ void xlu__cfg_set_free(XLU_ConfigSetting *set);
 void xlu__cfg_set_store(CfgParseContext*, char *name,
                         XLU_ConfigValue *val, int lineno);
 XLU_ConfigValue *xlu__cfg_string_mk(CfgParseContext *ctx,
-                                    char *atom);
+                                    char *atom, YYLTYPE *loc);
 XLU_ConfigValue *xlu__cfg_list_mk(CfgParseContext *ctx,
-                                  XLU_ConfigValue *val);
+                                  XLU_ConfigValue *val,
+                                  YYLTYPE *loc);
 void xlu__cfg_list_append(CfgParseContext *ctx,
                           XLU_ConfigValue *list,
                           XLU_ConfigValue *val);
diff --git a/tools/libxl/libxlu_cfg_y.c b/tools/libxl/libxlu_cfg_y.c
index b05e48b..fbfdd0f 100644
--- a/tools/libxl/libxlu_cfg_y.c
+++ b/tools/libxl/libxlu_cfg_y.c
@@ -1515,7 +1515,7 @@ yyreduce:
 
 /* Line 1806 of yacc.c  */
 #line 62 "libxlu_cfg_y.y"
-    { (yyval.value)= xlu__cfg_string_mk(ctx,(yyvsp[(1) - (1)].string)); }
+    { (yyval.value)= xlu__cfg_string_mk(ctx,(yyvsp[(1) - 
(1)].string),&(yylsp[(1) - (1)])); }
     break;
 
   case 13:
@@ -1543,7 +1543,7 @@ yyreduce:
 
 /* Line 1806 of yacc.c  */
 #line 68 "libxlu_cfg_y.y"
-    { (yyval.value)= xlu__cfg_list_mk(ctx,NULL); }
+    { (yyval.value)= xlu__cfg_list_mk(ctx,NULL,&yylloc); }
     break;
 
   case 17:
@@ -1564,7 +1564,7 @@ yyreduce:
 
 /* Line 1806 of yacc.c  */
 #line 72 "libxlu_cfg_y.y"
-    { (yyval.value)= xlu__cfg_list_mk(ctx,(yyvsp[(1) - (2)].value)); }
+    { (yyval.value)= xlu__cfg_list_mk(ctx,(yyvsp[(1) - (2)].value),&(yylsp[(1) 
- (2)])); }
     break;
 
   case 20:
diff --git a/tools/libxl/libxlu_cfg_y.y b/tools/libxl/libxlu_cfg_y.y
index 4a5ca3a..a923f76 100644
--- a/tools/libxl/libxlu_cfg_y.y
+++ b/tools/libxl/libxlu_cfg_y.y
@@ -59,17 +59,17 @@ assignment: IDENT '=' value { 
xlu__cfg_set_store(ctx,$1,$3,@3.first_line); }
 endstmt: NEWLINE
  |      ';'
 
-value:  atom                         { $$= xlu__cfg_string_mk(ctx,$1); }
+value:  atom                         { $$= xlu__cfg_string_mk(ctx,$1,&@1); }
  |      '[' nlok valuelist ']'       { $$= $3; }
 
 atom:   STRING                   { $$= $1; }
  |      NUMBER                   { $$= $1; }
 
-valuelist: /* empty */           { $$= xlu__cfg_list_mk(ctx,NULL); }
+valuelist: /* empty */           { $$= xlu__cfg_list_mk(ctx,NULL,&yylloc); }
  |      values                  { $$= $1; }
  |      values ',' nlok         { $$= $1; }
 
-values: value nlok                  { $$= xlu__cfg_list_mk(ctx,$1); }
+values: value nlok                  { $$= xlu__cfg_list_mk(ctx,$1,&@1); }
  |      values ',' nlok value nlok  { xlu__cfg_list_append(ctx,$1,$4); $$= $1; 
}
 
 nlok:
diff --git a/tools/libxl/libxlu_internal.h b/tools/libxl/libxlu_internal.h
index 092a17a..cc1d400 100644
--- a/tools/libxl/libxlu_internal.h
+++ b/tools/libxl/libxlu_internal.h
@@ -38,12 +38,14 @@ typedef struct XLU_ConfigList {
     XLU_ConfigValue **values;
 } XLU_ConfigList;
 
+typedef struct YYLTYPE YYLTYPE;
 struct XLU_ConfigValue {
     enum XLU_ConfigValueType type;
     union {
         char *string;
         XLU_ConfigList list;
     } u;
+    YYLTYPE *loc;
 };
 
 typedef struct XLU_ConfigSetting { /* transparent */
-- 
1.9.1


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.