Re: [Squashfs-devel] [PATCH 1/2] Squashfs: add LZO decompression support

Re: [Squashfs-devel] [PATCH 1/2] Squashfs: add LZO decompression support

[Phillip Lougher]

Chan Jeong wrote:
> Hi,
> 
> This patch series adds LZO decompression support to Squashfs,
> using in-kernel LZO decompression library and Squashfs 
> decompressor entries.
> 
> The patch series also includes LZO support for Squashfs tools.
> 
> Signed-off-by: Chan Jeong <chan.jeong@lge.com>

Hi Chan

Thanks for the patches.  How much testing have these patches had?
 From an eyeball of the code (and a check of the LZO decompressor
code in the kernel) they look OK.

I'm currently considering whether to push these for mainline in
the current 2.6.35 merge window.  I'd like to push them in this
merge window, however, pushing them now is definitely cutting things
fine because they were only posted for review after the merge window
opened (and the established etiquette is the merge window is to push
things that were posted for review and ideally in linux-next well
before the merge window opened).

So, some idea of how much testing these patches have had would
increase my confidence about pushing them in this merge window.

Thanks

Phillip

Re: [PATCH 1/2] Squashfs: add LZO decompression support

[Chan Jeong]

> Chan Jeong wrote:
> > Hi,
> >
> > This patch series adds LZO decompression support to Squashfs,
> > using in-kernel LZO decompression library and Squashfs
> > decompressor entries.
> >
> > The patch series also includes LZO support for Squashfs tools.
> >
> > Signed-off-by: Chan Jeong <chan.jeong-Hm3cg6mZ9cc@public.gmane.org>
> 
> Hi Chan
> 
> Thanks for the patches.  How much testing have these patches had?
>  From an eyeball of the code (and a check of the LZO decompressor
> code in the kernel) they look OK.
> 
> I'm currently considering whether to push these for mainline in
> the current 2.6.35 merge window.  I'd like to push them in this
> merge window, however, pushing them now is definitely cutting things
> fine because they were only posted for review after the merge window
> opened (and the established etiquette is the merge window is to push
> things that were posted for review and ideally in linux-next well
> before the merge window opened).
> 
> So, some idea of how much testing these patches have had would
> increase my confidence about pushing them in this merge window.
> 
> Thanks
> 
> Phillip

Hi Phillip,

Thank you for your feedback. I have done much testing with those
patches on X86 PCs and a MIPS-based TV product.

Actually the TV product has been working fine with Squashfs-zlib
filesystem images, and after replacement with lzo images, it is
running good for long-run test and all TV functions such as video,
audio, UI and networking are working fine as well.

And Here is just clean-up for the first patch. The kcalloc() is
safer for lzo workspace pointer array, because it returns zeroed
memory.

Thanks,
Chan Jeong

--
diff --git a/fs/squashfs/lzo_wrapper.c b/fs/squashfs/lzo_wrapper.c
index 6577cfa..02a767a 100644
--- a/fs/squashfs/lzo_wrapper.c
+++ b/fs/squashfs/lzo_wrapper.c
@@ -38,7 +38,7 @@ static void *lzo_init(struct squashfs_sb_info *msblk)
 {
 	void **streams;
 
-	streams = (void **)kmalloc(sizeof(void *) * 2, GFP_KERNEL);
+	streams = (void **)kcalloc(2, sizeof(void *), GFP_KERNEL);
 	if (streams == NULL)
 		goto failed;
 
@@ -56,11 +56,9 @@ static void *lzo_init(struct squashfs_sb_info *msblk)
 
 failed:
 	ERROR("Failed to allocate lzo workspace\n");
-	if (streams) {
-		if (streams[0])
-			vfree(streams[0]);
-		kfree(streams);
-	}
+	if (streams)
+		vfree(streams[0]);
+	kfree(streams);
 	return NULL;
 }
 
@@ -68,8 +66,11 @@ failed:
 static void lzo_free(void *stream)
 {
 	void **streams = (void **)stream;
-	vfree(streams[1]);
-	vfree(streams[0]);
+
+	if (streams) {
+		vfree(streams[1]);
+		vfree(streams[0]);
+	}
 	kfree(streams);
 }
 


------------------------------------------------------------------------------