topic Re: suspicious code in JPEG Samples for Windows in Intel® Integrated Performance Primitives

suspicious code in JPEG Samples for Windows

goldfita — Wed, 31 Jan 2007 05:36:09 GMT

I have been working a bit with the IPP JPEG samples.

Package ID: w_ipp-sample-jpeg_p_5.1.1.001
Package Contents: Intel IPP JPEG Samples for Windows*

I noticed the following code in enchtbl.cpp always copies 256 bytes even though the arrays passed to it are less than that length.

ippsCopy_8u(vals,m_vals,256);

By stepping through the code, it's clear that m_vals is getting not just vals copied to it, but the memory following that as well. This does not seem to be causing a problem; although, it might if different compiler options were used.

I don't need a response to this, thanks.

Re: suspicious code in JPEG Samples for Windows

Vladimir_Dudnik — Wed, 31 Jan 2007 08:02:46 GMT

Thank for informing about that. The m_vals is defined in enchtbl.h as an array of 256 Ipp8u, so it should not be overwtitten with copying 256 values into it.

I'm agree that interface for CJPEGEncoderHuffmanTable::Init() should define the length of input vals and bits. Better if change to something like that

JERRCODE CJPEGEncoderHuffmanTable::Init(
int id, int hclass, Ipp8u bits[16], Ipp8u vals[256])

Number 256 itself is maximum possible number of huffman values. In codec we always allocate memory enough for maximum possible number of values, even if some memory will not be used in real application.

Regards,
Vladimir

Re: suspicious code in JPEG Samples for Windows

goldfita — Wed, 31 Jan 2007 19:14:34 GMT

The problem is vals, not m_vals. Take a look at what is coming into the function (DefaultLuminanceDCValues).

const

Ipp8u DefaultLuminanceDCValues[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,0x08, 0x09, 0x0a, 0x0b};

The program does not crash because the copy function reads into memory within the program's data space.

Re: suspicious code in JPEG Samples for Windows

Vladimir_Dudnik — Wed, 31 Jan 2007 21:54:13 GMT

Oh my goodness! You are absolutely right, that is a bug which should be (and will be) fixed.

Thanks for catching that!

Regards,
Vladimir