| Message ID | 1461682343-20597-8-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 26, 2016 at 04:52:16PM +0200, Roger Pau Monne wrote: > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > Cc: Wei Liu <wei.liu2@citrix.com> > --- > tools/libxc/xc_dom_bzimageloader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c > index 7fde42a..0a4041c 100644 > --- a/tools/libxc/xc_dom_bzimageloader.c > +++ b/tools/libxc/xc_dom_bzimageloader.c > @@ -482,7 +482,7 @@ static int xc_try_lzo1x_decode( > if ( !dst_len ) > { > msg = "Error registering stream output"; > - if ( xc_dom_register_external(dom, out_buf, out_len) ) > + if ( xc_dom_register_external(dom, out_buf, 0) ) I fail to figure out why this is correct. I think the out_len should be moved out of the loop and initialise as 0. We still need to use out_len here. Wei. > break; > > return 0; > -- > 2.6.4 (Apple Git-63) >
On Tue, Apr 26, 2016 at 04:16:51PM +0100, Wei Liu wrote: > On Tue, Apr 26, 2016 at 04:52:16PM +0200, Roger Pau Monne wrote: > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > Cc: Wei Liu <wei.liu2@citrix.com> > > --- > > tools/libxc/xc_dom_bzimageloader.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c > > index 7fde42a..0a4041c 100644 > > --- a/tools/libxc/xc_dom_bzimageloader.c > > +++ b/tools/libxc/xc_dom_bzimageloader.c > > @@ -482,7 +482,7 @@ static int xc_try_lzo1x_decode( > > if ( !dst_len ) > > { > > msg = "Error registering stream output"; > > - if ( xc_dom_register_external(dom, out_buf, out_len) ) > > + if ( xc_dom_register_external(dom, out_buf, 0) ) > > I fail to figure out why this is correct. > > I think the out_len should be moved out of the loop and initialise as 0. > We still need to use out_len here. I'm not following here. AFAICT out_len is always uninitialized at this point (from loop 0 to N), so I assume 0 is what was intended to be passed here. Or it this supposed to be passing in the last out_len value from the previous iteration of the loop? Roger.
On 27/04/16 09:57, Roger Pau Monne wrote: > On Tue, Apr 26, 2016 at 04:16:51PM +0100, Wei Liu wrote: >> On Tue, Apr 26, 2016 at 04:52:16PM +0200, Roger Pau Monne wrote: >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> --- >>> Cc: Ian Jackson <ian.jackson@eu.citrix.com> >>> Cc: Wei Liu <wei.liu2@citrix.com> >>> --- >>> tools/libxc/xc_dom_bzimageloader.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c >>> index 7fde42a..0a4041c 100644 >>> --- a/tools/libxc/xc_dom_bzimageloader.c >>> +++ b/tools/libxc/xc_dom_bzimageloader.c >>> @@ -482,7 +482,7 @@ static int xc_try_lzo1x_decode( >>> if ( !dst_len ) >>> { >>> msg = "Error registering stream output"; >>> - if ( xc_dom_register_external(dom, out_buf, out_len) ) >>> + if ( xc_dom_register_external(dom, out_buf, 0) ) >> I fail to figure out why this is correct. >> >> I think the out_len should be moved out of the loop and initialise as 0. >> We still need to use out_len here. > I'm not following here. AFAICT out_len is always uninitialized at this > point (from loop 0 to N), so I assume 0 is what was intended to be passed > here. Or it this supposed to be passing in the last out_len value from the > previous iteration of the loop? xc_dom_register_external() is completely mad, and it is not obvious as to its purpose; tt is doing some kind of allocation tracking for malloc()-allocated blocks. It looks like the output length is only uses for stats purposes, as the memory in out_buf will be freed properly by free(). So I think the correct value should be the actual size of the allocation of out_buf, but I also don't think it makes any real difference either way. ~Andrew
On Wed, Apr 27, 2016 at 10:57:17AM +0200, Roger Pau Monne wrote: > On Tue, Apr 26, 2016 at 04:16:51PM +0100, Wei Liu wrote: > > On Tue, Apr 26, 2016 at 04:52:16PM +0200, Roger Pau Monne wrote: > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > > --- > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com> > > > Cc: Wei Liu <wei.liu2@citrix.com> > > > --- > > > tools/libxc/xc_dom_bzimageloader.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c > > > index 7fde42a..0a4041c 100644 > > > --- a/tools/libxc/xc_dom_bzimageloader.c > > > +++ b/tools/libxc/xc_dom_bzimageloader.c > > > @@ -482,7 +482,7 @@ static int xc_try_lzo1x_decode( > > > if ( !dst_len ) > > > { > > > msg = "Error registering stream output"; > > > - if ( xc_dom_register_external(dom, out_buf, out_len) ) > > > + if ( xc_dom_register_external(dom, out_buf, 0) ) > > > > I fail to figure out why this is correct. > > > > I think the out_len should be moved out of the loop and initialise as 0. > > We still need to use out_len here. > > I'm not following here. AFAICT out_len is always uninitialized at this > point (from loop 0 to N), so I assume 0 is what was intended to be passed > here. Or it this supposed to be passing in the last out_len value from the > previous iteration of the loop? > Note that out_buf is defined out of the scope of this loop, which points to the buffer returns to caller when decompression completes (note the return 0 in that block). The second argument to xc_dom_register_external should reflect the true size of the buffer. I got confused by the name of out_len because it is actually used for something else. So please ignore what I said in my earlier email about using out_len. From reading the code I think we need to use *size or something. Does this make sense? Wei. > Roger.
diff --git a/tools/libxc/xc_dom_bzimageloader.c b/tools/libxc/xc_dom_bzimageloader.c index 7fde42a..0a4041c 100644 --- a/tools/libxc/xc_dom_bzimageloader.c +++ b/tools/libxc/xc_dom_bzimageloader.c @@ -482,7 +482,7 @@ static int xc_try_lzo1x_decode( if ( !dst_len ) { msg = "Error registering stream output"; - if ( xc_dom_register_external(dom, out_buf, out_len) ) + if ( xc_dom_register_external(dom, out_buf, 0) ) break; return 0;
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- tools/libxc/xc_dom_bzimageloader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)