Re: [PATCH 1/2] xen: move perform_gunzip to common

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-08-13 at 03:57 -0600, Jan Beulich wrote:
> > 
> > > > On 13.08.15 at 11:28, <stefano.stabellini@eu.citrix.com> wrote:
> > On Thu, 13 Aug 2015, Jan Beulich wrote:
> > > > > > On 12.08.15 at 18:15, <stefano.stabellini@eu.citrix.com> wrote:
> > > > On Wed, 12 Aug 2015, Jan Beulich wrote:
> > > > > > > > On 12.08.15 at 16:47, <stefano.stabellini@eu.citrix.com> 
> > > > > > > > wrote:
> > > > > > @@ -31,8 +33,15 @@ typedef int decompress_fn(unsigned char 
> > > > > > *inbuf, unsigned int len,
> > > > > >   * dependent).
> > > > > >   */
> > > > > >  
> > > > > > -decompress_fn bunzip2, unxz, unlzma, unlzo, unlz4;
> > > > > > +decompress_fn perform_gunzip, bunzip2, unxz, unlzma, unlzo, 
> > > > > > unlz4;
> > > > > >  
> > > > > >  int decompress(void *inbuf, unsigned int len, void *outbuf);
> > > > > >  
> > > > > > +static inline unsigned long output_length(char *image, 
> > > > > > unsigned long image_len)
> > > > > 
> > > > > Neither of the callers gets moved out of bzimage.c - why does 
> > > > > this
> > > > > function need to move?
> > > > 
> > > > We'll use it on arm.
> > > 
> > > Hmm, the way it is used on x86 makes it quite architecture specific
> > > (namely because of the assumption that the size is also in said
> > > place for non-gz compression methods). I'd therefore prefer code
> > > duplication over code sharing here. 
> > 
> > Actually after seeing the size and quality of the resulting patches, I
> > am starting to feel the same way.
> > 
> > In terms of code changes, I was thinking that the best result would be
> > moving the "boilerplate" code from xen/arch/x86/bzimage.c to
> > xen/common/inflate.c, see below, then the interface would become just
> > perform_gunzip and gzip_check. But I guess you wouldn't want inflate.c
> > to be modified, right?
> 
> Yes, unless really unavoidable.
> 
> > Alternatively we could move it to a new file, let's call it gunzip.h,
> > that would #include "inflate.c", so:
> > 
> > bzimage.c -- #include --> gunzip.h -- #include --> inflate.c
> > 
> > And again we just leave the perform_gunzip and gzip_check calls in
> > bzimage.c.  What do you think?

How about putting perform_gunzip and gzip_check into a new gunzip.c which
includes inflate.c?