Mailing List Archive: [PATCH V2 02/11] libxl_json, Check the parser status before to call parse

[PATCH V2 02/11] libxl_json, Check the parser status before to call parse_complete

Oct 20, 2011, 10:59 AM

Post #1 of 3 (183 views)

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
tools/libxl/libxl_json.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
index 11f65fc..c743114 100644
--- a/tools/libxl/libxl_json.c
+++ b/tools/libxl/libxl_json.c
@@ -744,7 +744,9 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
yajl_ctx.hand = yajl_alloc(&callbacks, &cfg, NULL, &yajl_ctx);
}
status = yajl_parse(yajl_ctx.hand, (const unsigned char *)s, strlen(s));
- status = yajl_parse_complete(yajl_ctx.hand);
+ if (status == yajl_status_ok) {
+ status = yajl_parse_complete(yajl_ctx.hand);
+ }

if (status == yajl_status_ok) {
libxl__json_object *o = yajl_ctx.head;
--
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH V2 02/11] libxl_json, Check the parser status before to call parse_complete [ In reply to ]

Ian.Campbell at citrix

Oct 24, 2011, 8:01 AM

Post #2 of 3 (180 views)

Permalink

On Mon, 2011-10-24 at 15:59 +0100, Anthony PERARD wrote:
> On Mon, Oct 24, 2011 at 10:38, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Thu, 2011-10-20 at 18:59 +0100, Anthony PERARD wrote:
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >> tools/libxl/libxl_json.c | 4 +++-
> >> 1 files changed, 3 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/tools/libxl/libxl_json.c b/tools/libxl/libxl_json.c
> >> index 11f65fc..c743114 100644
> >> --- a/tools/libxl/libxl_json.c
> >> +++ b/tools/libxl/libxl_json.c
> >> @@ -744,7 +744,9 @@ libxl__json_object *libxl__json_parse(libxl__gc *gc, const char *s)
> >> yajl_ctx.hand = yajl_alloc(&callbacks, &cfg, NULL, &yajl_ctx);
> >> }
> >> status = yajl_parse(yajl_ctx.hand, (const unsigned char *)s, strlen(s));
> >> - status = yajl_parse_complete(yajl_ctx.hand);
> >> + if (status == yajl_status_ok) {
> >> + status = yajl_parse_complete(yajl_ctx.hand);
> >> + }
> >>
> >> if (status == yajl_status_ok) {
> >
> > You now have two of these checks back-to-back. I guess they could be
> > combined?
>
> :(, I do not see how I could combine them. Because, if I call
> parse_complete() after a parse() fail, I do not see a good error
> message(yajl forget a bit of his status). And parse_complete() could
> fail too, so I want to check is status too.
> So, I check the same status twice, only in case of error.
>
> (the second if(status) will print the error messages.)

I totally missed that status was updated inside the first if, sorry!

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH V2 02/11] libxl_json, Check the parser status before to call parse_complete [ In reply to ]

Ian.Jackson at eu

Oct 25, 2011, 10:27 AM

Post #3 of 3 (175 views)

Permalink

Anthony PERARD writes ("[Xen-devel] [PATCH V2 02/11] libxl_json, Check the parse> - status = yajl_parse_complete(yajl_ctx.hand);
> + if (status == yajl_status_ok) {
> + status = yajl_parse_complete(yajl_ctx.hand);
> + }

Perhaps this would be better done with the "goto out" pattern ?

Ie:

status = yajl_do_something();
if (status != yajl_status_ok) goto out;

status = ....

....

return o;

out:
blah blah get_error blah blah
return NULL;

That would make the logic clearer I think.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel