From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH 1/2] xen: move perform_gunzip to common
Date: Thu, 13 Aug 2015 11:17:50 +0100
Message-ID: <1439461070.23981.61.camel@citrix.com>
References: <alpine.DEB.2.02.1508121544010.11337@kaball.uk.xensource.com>
	<1439390875-21256-1-git-send-email-stefano.stabellini@eu.citrix.com>
	<55CB7EF3020000780009A35C@prv-mh.provo.novell.com>
	<alpine.DEB.2.02.1508121713010.22657@kaball.uk.xensource.com>
	<55CC5573020000780009A6E3@prv-mh.provo.novell.com>
	<alpine.DEB.2.02.1508131016170.22657@kaball.uk.xensource.com>
	<55CC862C020000780009A924@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=66097562a=Ian.Campbell@citrix.com>)
	id 1ZPpa6-0002lR-Ha
	for xen-devel@lists.xenproject.org; Thu, 13 Aug 2015 10:17:54 +0000
In-Reply-To: <55CC862C020000780009A924@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, StefanoStabellini <stefano.stabellini@eu.citrix.com>
Cc: andrew.cooper3@citrix.com, xen-devel <xen-devel@lists.xenproject.org>
List-Id: xen-devel@lists.xenproject.org

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?