proper prototype for lzf_compress() on NetBSD

[GoogleCodeExporter]

lzf_compress has a hash parameter on NetBSD; find attached patch which 
compiles/runs. Tested NetBSD -current on AMD64.

Cheers,

-bch

Original issue reported on code.google.com by brad.har...@gmail.com on 29 Mar 2011 at 6:17

Attachments:

• patch-aa

[GoogleCodeExporter]

Hi,

I looked at the liblzf source code, but I'm unable to find anything 
NetBSD-specific there. Can you point me to more information about this?

Original comment by se...@google.com on 29 Mar 2011 at 8:33

[GoogleCodeExporter]

Note the extra param at end of call to lzf_compress. NetBSD takes a pointer to 
a hash that is optionally used (for low memory conditions). It appears macro 
LZF_STATE_ARG determines if this hash is used. I need to dig into this more to 
come to fully understand it, but didn't want to wait raising this issue here.

Look forward to hearing about your observations.

-bch

Original comment by brad.har...@gmail.com on 29 Mar 2011 at 6:52

[GoogleCodeExporter]

Again, where is your liblzf from? liblzf upstream has

  unsigned int
  lzf_compress (const void *const in_data,  unsigned int in_len,
                void             *out_data, unsigned int out_len);

No fifth parameter, not even in an #ifdef.

If something in NetBSD (NetBSD ports?) has chosen to break liblzf's API, I'm 
not entirely sure if Snappy upstream should follow suit, especially since one 
might want to use the upstream version instead. At the very least, there should 
be some sort of #define saying that a modified API is in use.

Original comment by se...@google.com on 29 Mar 2011 at 6:57

[GoogleCodeExporter]

lzf I'm talking about ships w/ NetBSD system.

kamloops$ uname -srm
NetBSD 5.99.48 amd64
kamloops$ head -n 81 /usr/include/lzf.h |tail -n 30

/*
 * Compress in_len bytes stored at the memory block starting at
 * in_data and write the result to out_data, up to a maximum length
 * of out_len bytes.
 *
 * If the output buffer is not large enough or any error occurs return 0,
 * otherwise return the number of bytes used, which might be considerably
 * more than in_len (but less than 104% of the original size), so it
 * makes sense to always use out_len == in_len - 1), to ensure _some_
 * compression, and store the data uncompressed otherwise (with a flag, of
 * course.
 *
 * lzf_compress might use different algorithms on different systems and
 * even different runs, thus might result in different compressed strings
 * depending on the phase of the moon or similar factors. However, all
 * these strings are architecture-independent and will result in the
 * original data when decompressed using lzf_decompress.
 *
 * The buffers must not be overlapping.
 *
 * If the option LZF_STATE_ARG is enabled, an extra argument must be
 * supplied which is not reflected in this header file. Refer to lzfP.h
 * and lzf_c.c.
 *
 */
unsigned int 
lzf_compress (const void *const in_data,  unsigned int in_len,
              void             *out_data, unsigned int out_len,
              LZF_STATE htab);

Original comment by brad.har...@gmail.com on 29 Mar 2011 at 8:22

[GoogleCodeExporter]

I talked to the liblzf author. Quote (some spelling mistakes fixed and 
punctuation added): “liblzf doesn't have this parameter. NetBSD has had to 
patch their copy; you can enable it when embedding liblzf. [...] I don't think 
Snappy should support that. [...] I think breaking the API for the default 
system-provided liblzf is just broken.”

So, end conclusion: If NetBSD wants to break the liblzf API and still expose it 
to userspace, they will also need to patch any callers to it. Either that, or 
you can just compile Snappy against unmodified upstream liblzf. In other words, 
I'm going to reject this bug as invalid.

Original comment by se...@google.com on 29 Mar 2011 at 11:10

• Changed state: Invalid

[GoogleCodeExporter]

I'll discuss w/ NetBSD folks. Thanks for your attention.

-bch

Original comment by brad.har...@gmail.com on 30 Mar 2011 at 1:06