free, rather than delete, malloc'd object.

free, rather than delete, malloc'd object.

BUG=381031
Original patch by Chris Palmer <palmer@chromium.org>;

Committed: https://pdfium.googlesource.com/pdfium/+/9e16edd

[palmer, 2014-06-05 22:59:54]

jam: PTAL. Thanks! This diff straightforwardly turns an immediate ASAN explosion
into normal non-crashing functionality, so I think it's OK.

[jam, 2014-06-05 23:50:23]

Also adding some Foxit developers so that they can ensure this gets merged to
their repo.

https://codereview.chromium.org/322453002/diff/1/core/src/fxcodec/jbig2/JBig2...
File core/src/fxcodec/jbig2/JBig2_Context.cpp (right):

https://codereview.chromium.org/322453002/diff/1/core/src/fxcodec/jbig2/JBig2...
core/src/fxcodec/jbig2/JBig2_Context.cpp:62: m_pModule->JBig2_Free(m_gbContext);
seems that this is a problem with other member variables in this class?

[palmer, 2014-06-06 19:03:16]

> seems that this is a problem with other member variables in this class?

Conter-intuitively, JBIG2_ALLOC boils down to new, not malloc. I checked all the
rest of the heap-allocated m_* members, and they seem to be new'd and delete'd
consistently. I guess the problem was specific to |m_gbContext|.

[jam, 2014-06-06 19:16:15]

On 2014/06/06 19:03:16, Chromium Palmer wrote:
> > seems that this is a problem with other member variables in this class?
> 
> Conter-intuitively, JBIG2_ALLOC boils down to new, not malloc. I checked all
the
> rest of the heap-allocated m_* members, and they seem to be new'd and delete'd
> consistently. I guess the problem was specific to |m_gbContext|.

got it, thanks for the explanation.

I defer to Foxit devs for review of the code.

[jun_fang, 2014-06-09 01:25:20]

On 2014/06/06 19:16:15, jam wrote:
> On 2014/06/06 19:03:16, Chromium Palmer wrote:
> > > seems that this is a problem with other member variables in this class?
> > 
> > Conter-intuitively, JBIG2_ALLOC boils down to new, not malloc. I checked all
> the
> > rest of the heap-allocated m_* members, and they seem to be new'd and
delete'd
> > consistently. I guess the problem was specific to |m_gbContext|.
> 
> got it, thanks for the explanation.
> 
> I defer to Foxit devs for review of the code.

I reviewed the code change. It's fine for this issue. Below is my reply for
Jam's comment.  

A different way, JBIG2_ALLOC, is used for memory allocation of the other member
variables in this de-construction. It is a wrapper of 'new'. It's fine to use
'delete' to release memory for them. The variable m_gbcontext in this issue
points to the memory allocated by JBig2_Malloc which is internally implemented
in PDF SDK. It needs to call JBig2_Free to release memory.

[jun_fang, 2014-06-09 01:26:25]

https://codereview.chromium.org/322453002/diff/1/core/src/fxcodec/jbig2/JBig2...
File core/src/fxcodec/jbig2/JBig2_Context.cpp (right):

https://codereview.chromium.org/322453002/diff/1/core/src/fxcodec/jbig2/JBig2...
core/src/fxcodec/jbig2/JBig2_Context.cpp:62: m_pModule->JBig2_Free(m_gbContext);
On 2014/06/05 23:50:23, jam wrote:
> seems that this is a problem with other member variables in this class?

A different way, JBIG2_ALLOC, is used for memory allocation of the other member
variables in this de-construction. It is a wrapper of 'new'. It's fine to use
'delete' to release memory for them. The variable m_gbcontext in this issue
points to the memory allocated by JBig2_Malloc which is internally implemented
in PDF SDK. It needs to call JBig2_Free to release memory.

[palmer, 2014-06-09 18:12:14]

Thank you, jun_fang!

[palmer, 2014-06-09 18:12:26]

The CQ bit was checked by palmer@chromium.org

[commit-bot: I haz the power, 2014-06-09 18:15:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-09 18:15:20]

Commit queue rejected this change because it did not recognize the base URL.
Please commit your change manually.

[palmer, 2014-06-09 18:26:30]

Oh, right. jam, can you please commit this now that jun_fang has signed off on
it? Thanks.