[BusyBox 0004944]: patch to support lzop compression/decompression

bugs at busybox.net bugs at busybox.net
Mon Sep 29 22:42:47 UTC 2008

A NOTE has been added to this issue. 
Reported By:                Alain137
Assigned To:                BusyBox
Project:                    BusyBox
Issue ID:                   4944
Category:                   New Features
Reproducibility:            N/A
Severity:                   feature
Priority:                   normal
Status:                     assigned
Date Submitted:             09-13-2008 00:54 PDT
Last Modified:              09-29-2008 15:42 PDT
Summary:                    patch to support lzop compression/decompression
This is a patch to add the lzop compressor/decompressor to busybox.
Lzop is a compressor that is more than twice as fast as gzip at its
fastest setting, and is thus useful in any application where
compression/decompression speed is more important than ratio. For
instance, I use lzop with udcpast to speed up network transfers (gzip
would actually make the transfer slower, as any time gain obtained due to
having less data to transfer would be lost due to the time it takes to


> time lzop -c /usr/bin/mplayer >/dev/null
lzop -c /usr/bin/mplayer > /dev/null  0,33s user 0,01s system 100% cpu
0,337 total
> time gzip -1 -c /usr/bin/mplayer >/dev/null
gzip -1 -c /usr/bin/mplayer > /dev/null  0,80s user 0,03s system 94% cpu
0,882 total

> time gzip -dc  mplayer.gz >/dev/null
gzip -dc mplayer.gz > /dev/null  0,27s user 0,01s system 97% cpu 0,288
> time lzop -dc mplayer.lzo >/dev/null
lzop -dc mplayer.lzo > /dev/null  0,10s user 0,01s system 100% cpu 0,112

For building it, a liblzo2 static library must be present on the system
(for example, liblzo2-dev on a Kubuntu system)


 vda - 09-20-08 07:56  
The patch seems to contain only the glue needed to interface with LZO
library. It does not contain LZO compression code itself. The "innermost"
routines are lzo_compress() and lzo_decompress() but from there they just
call into liblzo2 (lzo1x_1_15_compress() etc).

And this glue code is big. I mean, it's VERY BIG.

For comparison: look into bbunzip.c, unlzma glue code is less than one

static char* make_new_name_unlzma(char *filename)
        return make_new_name_generic(filename, "lzma");
static USE_DESKTOP(long long) int unpack_unlzma(void)
        return unpack_lzma_stream(STDIN_FILENO, STDOUT_FILENO);
int unlzma_main(int argc UNUSED_PARAM, char **argv)
        getopt32(argv, "cf");
        argv += optind;
        /* lzmacat? */
        if (applet_name[4] == 'c')
                option_mask32 |= OPT_STDOUT;
        return bbunpack(argv, make_new_name_unlzma, unpack_unlzma);

lzop glue code should be just added to this file in a similar way.


 Alain137 - 09-23-08 22:30  
Well, part of the problem is that the LZO library (apparently?) only
handles the actual compressed data, but not the header. So, header
handling is left to the application (glue) 

 bernhardf - 09-24-08 00:21  
I would strongly suggest to make sure that we can use minilzo instead of
the rather big "normal" lzo.

For reference:

 vda - 09-24-08 03:54  
I'm not against glue code itself. I am just saying it is way, way too big.

I understand that you will need the code like lzo_compress() and
lzo_decompress() functions to handle .lzo data format and interface it
with LZO library, but they are not big. The rest of 100k+ code in this
patch is not needed.

Start by adding .lzo handling to bbunzip.c which calls not-yet-existing
unpack_lzo_stream(), then implement that one by taking only those parts of
lzo code that you need. I bet this way it will be much smaller. 

 Alain137 - 09-27-08 12:17  
I've made a new version (see attachment), which is much smaller: 36899
bytes of source, rather than 178272.

I admit, 36899 is still big (for just glue code), but unfortunately, the
lzop library is rather barebones, and _only_ supplies (de)compression of
one block, and not much else. Interpretation of headers, chaining of
blocks, calculation of checksums need all to be managed by the

 vda - 09-27-08 17:37  
Looked at the code.

e_method() is dead code, it can never be called:

+static void e_method(int m)
+       fprintf(stderr,"%s: illegal method option -- %c\n",
+               applet_name, m & 255);
+       e_usage();
+int lzo_set_method(int level)
+       int l = level & 0xff;
+       int m = 0;
+       if (l < 1 || l > 9)
+               return -1;          /* not a LZO method */
+       case '1':
+       case '2':
+       case '3':
+       case '4':
+       case '5':
+       case '6':
+       case '7':
+       case '8':
+       case '9':
+               if (lzo_set_method(optc - '0'))
+                       e_method(optc);

+extern const char *opt_complementary;

#include <libbb.h> does this already, no?

+int lzop_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
+int lzop_main(int argc UNUSED_PARAM, char **argv)
+       int i;
+       char *options="cfvdt123456789CFVh";
+       opt_complementary="h-";
+        int r=getopt32(argv, options);
+        argv += optind;
+        /* lzopcat? */
+        if (applet_name[4] == 'c')
+                option_mask32 |= (OPT_STDOUT | OPT_DECOMPRESS);
+        /* unlzop? */
+        if (applet_name[0] == 'u')
+                option_mask32 |= OPT_DECOMPRESS;
+       /* By default, use method 3 */
+       lzo_set_method(3);

Use tabs for indentation. Above is a mix of tabs/spaces, which is visible
in the diff. 

 Alain137 - 09-28-08 02:12  
Uploaded a new version with the issues fixed, and a couple of other

 vda - 09-28-08 05:09  
busybox already have a functions analogous to:
read_buf() - full_read(0,...)
write_buf() - xwrite(1,...)
error() - bb_error_msg_and_die()
io_error() - bb_perror_msg_and_die()

Why lzo_set_method() is returning a value? It's always 0, thus carries no
useful information.

+               if (dst_len > MAX_BLOCK_SIZE) {
+                       error("lzop file corrupted");
+                       ok = 0;
+                       break;
+               }

error() exits, why here you have code after call to error()?

+       case 'V':
+               bb_error_msg("version: v%s, %s",
+                            LZOP_VERSION_STRING, LZOP_VERSION_DATE);
+               exit(0);
+               break;

We do not need to support ALL options. Options like -V, --version are
usually not implemented in busybox applets.

+               if (read_buf(b1, src_len) != (lzo_int) src_len)
+                       read_error();

We have xread() for this in busybox.

+static int exit_code = EXIT_OK;

Unused variable.

-LDLIBS         :=
+LDLIBS         := lzo2

You need to add this _conditionally_. Example:
ifeq ($(CONFIG_SELINUX),y)
LDLIBS += selinux sepol

+    lzo_uint32 f_adler32;
+    lzo_uint32 f_crc32;

still using spaces for indent here 

 Alain137 - 09-29-08 11:00  
Just a quick note to notify you of some new versions that I attached
Patch5 is the most recent and smallest. I took account your recent
suggestions (LIBS, busybox methods, indentation ...). Moreover, I merged
everything into a single C file, bringing down the size to 26790.



 bernhardf - 09-29-08 11:04  
Note that LOC of source is not really relevant as opposed to the output of
$ ./scripts/bloat-o-meter busybox_unstripped.old busybox_unstripped
where the .old is without your code.

can you please paste this?

 bernhardf - 09-29-08 12:11  
A few comments on the patch itself:
- the code should live in the archival/ directory
- preferably it would be under GPLv2 "or later"
- includes
  The first include in lzop.c has to be libbb.h, then your <lzo/.*.h>
  and then you can remove all includes from stdio.h down to sys/stat.h
- Since we are concerned about size, you should check building against
  (see previous reply; did you verify that?)
- everything in "/* options*/" should live in globals, but we can fix that
- calloc() should be xzalloc()
- lzo_decompress() if (b2) {b2=NULL;free(b2)} This can't be right.
- magic numbers: 0 == STDIN_FILENO, 1 == STDOUT_FILENO etc. Please use
these for
- check_magic() is only used once. Inline it into the single caller.
- init_compress_header: h->flags = (F_OS & F_OS_MASK) | (F_CS &
- do_compress odd prototype, you forgot "void"

 vda - 09-29-08 15:42  
Having entire separate lzop/* directory for just one *.c file doesn't make

"config LZOP" should default to "n" as every other applet does.

All other applets share usage.h, lzop is not that special to have separate

Other compressors have compressor/decompressor selectable *separately* -
it's possible to select bunzip2 but not bzip2. lzop should be similar.

#include <stdio.h>
#include <string.h>
#include <limits.h>
#include <sys/types.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/stat.h>

This all is already included by libbb.h

        lzo_byte * const b1 = calloc(1, block_size);
        lzo_byte * const b2 = calloc(1, MAX_COMPRESSED_SIZE(block_size));

Since you do not check for allocation errors anyway,
at least use xmalloc() - it will die with an error message
intead of a segfault.

        if (wrk_mem)

free(NULL) is safe, you don't need if() here.

                        if (b2) {
                                b2 = NULL;

Whats going on here??

Okay, I tried to build it, and due to -Wundef being used by busubox
I got gobs of errors from lzo2 headers. Okay, disabled that.
Then I've got these:

archival/lzop.c:237: error: function declaration isn't a prototype
archival/lzop.c: In function 'f_read8':
archival/lzop.c:237: error: old-style function definition
archival/lzop.c: At top level:
archival/lzop.c:247: error: function declaration isn't a prototype
archival/lzop.c: In function 'f_read16':
archival/lzop.c:247: error: old-style function definition
archival/lzop.c: At top level:
archival/lzop.c:257: error: function declaration isn't a prototype
archival/lzop.c: In function 'f_read32':
archival/lzop.c:257: error: old-style function definition
archival/lzop.c: At top level:
archival/lzop.c:706: error: function declaration isn't a prototype
archival/lzop.c: In function 'do_compress':
archival/lzop.c:706: error: old-style function definition
archival/lzop.c:707: error: unused variable 'ok'
archival/lzop.c: In function 'lzop_main':
archival/lzop.c:783: error: ISO C90 forbids mixed declarations and code
archival/lzop.c:783: error: unused variable 'r'

Ooookay, fixing it all up... see attached 5.patch 

Issue History 
Date Modified   Username       Field                    Change               
09-13-08 00:54  Alain137       New Issue                                    
09-13-08 00:54  Alain137       Status                   new => assigned     
09-13-08 00:54  Alain137       Assigned To               => BusyBox         
09-13-08 00:54  Alain137       File Added: lzop.diff                        
09-20-08 07:54  vda            Note Added: 0011714                          
09-20-08 07:56  vda            Note Edited: 0011714                         
09-23-08 22:30  Alain137       Note Added: 0011824                          
09-24-08 00:21  bernhardf      Note Added: 0011834                          
09-24-08 03:54  vda            Note Added: 0011844                          
09-27-08 12:11  Alain137       File Added: busybox-1.12.0-lzop.patch            
09-27-08 12:17  Alain137       Note Added: 0012284                          
09-27-08 17:37  vda            Note Added: 0012294                          
09-28-08 02:10  Alain137       File Added: busybox-1.12.0-lzop-3.patch          
09-28-08 02:12  Alain137       Note Added: 0012304                          
09-28-08 05:09  vda            Note Added: 0012314                          
09-28-08 15:35  Alain137       File Added: busybox-1.12.0-lzop-4.patch          
09-28-08 16:49  Alain137       File Added: busybox-1.12.0-lzop-5.patch          
09-29-08 11:00  Alain137       Note Added: 0012464                          
09-29-08 11:04  bernhardf      Note Added: 0012474                          
09-29-08 12:11  bernhardf      Note Added: 0012484                          
09-29-08 15:42  vda            Note Added: 0012494                          

More information about the busybox-cvs mailing list