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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.08.15 at 11:28, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> On Thu, 13 Aug 2015, Jan Beulich wrote:
>> >>> On 12.08.15 at 18:15, <stefano.stabellini@xxxxxxxxxxxxx> wrote:
>> > On Wed, 12 Aug 2015, Jan Beulich wrote:
>> >> >>> On 12.08.15 at 16:47, <stefano.stabellini@xxxxxxxxxxxxx> 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?

That's an option.

Jan


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