Need reviewer: a couple of hprof fixes for Parfait

[Kelly O'Hair]

Ok, I have to confess, I had to look at this hprof table logic more.

This generic "table" is used all over the place in hprof, so I had to dust off some brain cells.
The info structures are extra structures or extra "info" for each table element, and it's perfectly
valid for the info structure size to be 0. In particular, string tables have info_size==0.
But when dealing with those info_size==0 tables, the info data is never asked for or accessed,
if it was hprof would be seriously broken with null pointer accesses all over the place.
So I am not convinced we are fixing any kind of bug here, just shutting up a static analysis
tool that can't follow the non-obvious code flows.

Parfait is seeing a NULL pointer returned here, and assumes that all calls to get_info could
return a NULL pointer, that is in fact wrong, because if info_size==0, this call will never be made.
Parfait doesn't know that. Not unexpected. And even without this check, if info_size==0, it
will return a NULL pointer anyway.

So I have changed my fix.

The static function get_info() in hprof_table.c is only called twice in that file, once by the
extern function table_get_info(), and once by a table walker.
I have removed the "return NULL;" from get_info() and made the checks where it is called instead.
I think this will remove the Parfait errors, but may generate some new ones, so we could be playing
a bit of a whack-a-mole game here.

New webrev was generated and I displaced the old one:
   http://cr.openjdk.java.net/~ohair/openjdk8/parfait_hprof_fixes/webrev/

Generic data structures where a pointer can be NULL sometimes and not-NULL other times
is not something Parfait (or perhaps any static analysis tool) can deal with very well.

-kto

On Jun 12, 2012, at 8:06 AM, Kelly Ohair wrote:

> if size is zero  it returns null
> thats the cascading error that parfait is finding
> asserts turn into nothing with product builds so i need a dead stop here to shut up parfait :(
> 
> 
> Sent from my iPhone
> 
> On Jun 11, 2012, at 22:56, David Holmes <david.holmes at oracle.com> wrote:
> 
>> On 12/06/2012 12:48 PM, Kelly O'Hair wrote:
>>> 
>>> Need reviewer.
>>> 
>>> I was asked to look at some Parfait errors in hprof code:
>>> 
>>> 7176138: Fixes for missing close() calls and possible null pointer reference instead of fatal error
>>> http://cr.openjdk.java.net/~ohair/openjdk8/parfait_hprof_fixes/webrev/
>> 
>> 
>> hprof_table.c
>> 
>> 211     if ( ltable->info_size == 0 ) {
>> 212         HPROF_ERROR(JNI_TRUE, "Table is empty and should never be.");
>> 213         return NULL;
>> 
>> It is not obvious this should be a fatal error. Elsewhere this is handled as an assert.
>> 
>> David