Custom zlib hook seems to leak memory

[lilith]

Given the following code

fn encode(){
//....
    lode.encoder.zlibsettings.custom_zlib = Some(zlib_compress_adapter);
}

extern "C" {
    /// zlib
    fn compress2(dest: *mut u8, dest_len: &mut c_ulong, source: *const u8, source_len: c_ulong, level: c_int) -> c_int;
}

unsafe extern "C" fn zlib_compress_adapter(dest: &mut *mut u8, dest_size: &mut usize, source: *const u8, source_size: usize, info: *const lodepng::CompressSettings) -> c_uint {
    assert!(dest.is_null());
    let dest_buf_size = source_size * 1001/1000 + 12;
    *dest = libc::malloc(dest_buf_size) as *mut u8;
    let mut compressed_size = dest_buf_size as c_ulong;
    compress2(*dest, &mut compressed_size, source, source_size as c_ulong, 6);
    *dest_size = compressed_size as usize;
    0
}

Valgrind reports that the memory allocated by libc::malloc is never freed.

Is lodepng supposed to be freeing the memory behind the dest pointer?

[kornelski]

Yes, lodepng is supposed to be freeing dest.

I'm tempted to change these adapters to use &mut Vec<u8> or even io::Write instead, so that something more rusty and less leaky could be used.

[kornelski]

Please try 3.0.0-alpha.1

BTW, https://docs.rs/flate2/1.0.16/flate2/write/index.html has methods that work on io::Write. It also has a feature flag for cloudflare_zlib for AVX-accelerated encoding.

[lilith]

I tried using it with zlib like this, but it produced corrupted png files:

fn zlib_compressor_6(input: &[u8], output: &mut dyn std::io::Write, context: &CompressSettings) -> std::result::Result<(), lodepng::Error> {
    zlib_compressor(input, output, context, 6)
}
fn zlib_compressor_9(input: &[u8], output: &mut dyn std::io::Write, context: &CompressSettings) -> std::result::Result<(), lodepng::Error> {
    zlib_compressor(input, output, context, 9)
}
fn zlib_compressor(input: &[u8], output: &mut dyn std::io::Write, context: &CompressSettings, zlib_level: i32) -> std::result::Result<(), lodepng::Error>{
    let target_buf_size = input.len() * 1001/1000 + 12;
    let mut target = Vec::with_capacity(target_buf_size);
    let mut compressed_size = target_buf_size as c_ulong;
    unsafe {
        compress2(target.as_mut_ptr(), &mut compressed_size, input.as_ptr(), input.len() as c_ulong, zlib_level);
    }
    target.truncate(compressed_size as usize);
    if let Err(e) = output.write(&target){
        return Err(lodepng::Error::new(1008));
    }
    Ok(())
}

[lilith]

I also tried this with the same result:

fn zlib_compressor(input: &[u8], output: &mut dyn std::io::Write, context: &CompressSettings, zlib_level: u32) -> std::result::Result<(), lodepng::Error>{
    let mut compress = flate2::write::ZlibEncoder::new(output, flate2::Compression::new(zlib_level));
    if let Err(e) = compress.write(&input){
        return Err(lodepng::Error::new(1008));
    }
    if let Err(e) = compress.flush_finish(){
        return Err(lodepng::Error::new(1009));
    }
    Ok(())
}

What code are you using with this hook?

[kornelski]

Almost the same, but I'm calling finish()?

[kornelski]

Yeah, flush finish doesn't end the stream:

This means that obtained byte array can by extended by another deflated stream. To close the stream add the two bytes 0x3 and 0x0.

[lilith]

I tried switching to .finish(), but I get the same LibPNG error: Not enough image data error when re-reading the written PNG.

[kornelski]

Oh, make sure to use write_all instead of write. The latter writes only as much as it likes to, not what you've told it to write.

[lilith]

That was it. Now I just need to figure out how to get both lodepng 3.0 and 2.6 to compile in the same assembly (DSSIM uses 2.6). Getting multiple definition of lodepng_add_itext'` etc for all extern C functions.

[kornelski]

3.0 and 2.6 won't compile together, because they both export a C API. The other crates will have to upgrade.

[kornelski]

I've released lodepng 3.0.0 stable, and updated dssim to use it.

[lilith]

Thank you so much!

…

On Mon, Jul 6, 2020, 1:05 PM Kornel ***@***.***> wrote: I've released lodepng 3.0.0 stable, and updated dssim to use it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#44 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2LH2VBR3TIKMRF6HHAIDR2IN75ANCNFSM4OOEK5JQ> .