Open GoogleCodeExporter opened 9 years ago
Also, if e->_val is not null here, it will leak as well:
FRISO_API void hash_put_mapping(
friso_hash_t _hash,
fstring key,
void * value )
{
uint_t bucket = ( key == NULL ) ? 0 : hash( key, _hash->length );
hash_entry_t e = *( _hash->table + bucket );
//check the given key is already exists or not.
for ( ; e != NULL; e = e->_next )
{
if ( key == e->_key
|| ( key != NULL && e->_key != NULL
&& strcmp( key, e->_key ) == 0 ) )
{
// ====> Missing Free here!
// if (e->_val) FRISO_FREE(e->_val);
e->_val = value;
return;
}
}
//put a new mapping into the hashtable.
_hash->table[bucket] = new_hash_entry( key, value, _hash->table[bucket] );
_hash->size++;
//check the condition to rebuild the hashtable.
if ( _hash->size >= _hash->threshold )
rebuild_hash( _hash );
}
Original comment by mle...@gmail.com
on 28 Jan 2014 at 8:30
And finally, there seems to be multiple leaks caused by this line:
FRISO_API void friso_dic_load
...
_word = string_copy_heap( _buffer, strlen(_buffer) );
...
There seems to be missing multiple 'frees'. One of them would be:
if ( ! ( lex == __LEX_ECM_WORDS__ || lex == __LEX_CEM_WORDS__ )
&& strlen( _word ) > length ) {
// ====> Missing Free here!
// FRISO_FREE(_word);
continue;
}
Original comment by mle...@gmail.com
on 28 Jan 2014 at 8:46
Theses are only what I think the leaks comes from.. It might not be the right
way to fix them.
Original comment by mle...@gmail.com
on 28 Jan 2014 at 9:16
In fact, I think the best way to fix this would be to call
default_fdic_callback from
FRISO_API void hash_put_mapping(
friso_hash_t _hash,
fstring key,
void * value )
{
uint_t bucket = ( key == NULL ) ? 0 : hash( key, _hash->length );
hash_entry_t e = *( _hash->table + bucket );
//check the given key is already exists or not.
for ( ; e != NULL; e = e->_next )
{
if ( key == e->_key
|| ( key != NULL && e->_key != NULL
&& strcmp( key, e->_key ) == 0 ) )
{
// ====> Missing Free here!
//e->_key = key;
//if ( _hash->fentry_func != NULL ) _hash->fentry_func(e);
e->_val = value;
return;
}
}
//put a new mapping into the hashtable.
_hash->table[bucket] = new_hash_entry( key, value, _hash->table[bucket] );
_hash->size++;
//check the condition to rebuild the hashtable.
if ( _hash->size >= _hash->threshold )
rebuild_hash( _hash );
}
We would need to modify hash struct:
typedef struct {
uint_t length;
uint_t size;
float factor;
uint_t threshold;
hash_entry_t *table;
// ===> fhash_callback_fn_t fentry_func;
} friso_hash_cdt;
and pass it at hash table creation like that:
dic[t] = new_hash_table(default_fdic_callback);
Original comment by mle...@gmail.com
on 28 Jan 2014 at 9:23
[deleted comment]
[deleted comment]
I backed home really early this year, and i live in the countryside without
network condition.
Thanks for you feedback first and terribly sorry to responsed you so late.
I've put the source code of friso on http://git.oschina.net/lionsoul/friso, and
you are really welcome to post you request and contribute code there!
Original comment by chenxin6...@gmail.com
on 5 Feb 2014 at 2:45
i've checked the code, the 1th and the 3th issue you posted will do cause
memory leak.
As for the hash_put_mapping function in friso_lexicon.c, you can't just invoke
function free to release the allocation of e->_val like you posted: "// if
(e->_val) FRISO_FREE(e->_val);"
Cause the hash API was design for underlying use and you could never know the
real struct of e->_val, i mean we could change it to return the e->_val, and
let the developer to release it and it will be like this:
FRISO_API void *hash_put_mapping(
friso_hash_t _hash,
fstring key,
void * value )
{
uint_t bucket = ( key == NULL ) ? 0 : hash( key, _hash->length );
hash_entry_t e = *( _hash->table + bucket );
void *oval = NULL;
//check the given key is already exists or not.
for ( ; e != NULL; e = e->_next )
{
if ( key == e->_key
|| ( key != NULL && e->_key != NULL
&& strcmp( key, e->_key ) == 0 ) )
{
oval = e->_val;
e->_val = value;
return oval;
}
}
//put a new mapping into the hashtable.
_hash->table[bucket] = new_hash_entry( key, value, _hash->table[bucket] );
_hash->size++;
//check the condition to rebuild the hashtable.
if ( _hash->size >= _hash->threshold )
rebuild_hash( _hash );
return oval;
}
Original comment by chenxin6...@gmail.com
on 5 Feb 2014 at 2:45
Hi, I'm glad yo came back!
Indeed, we cannot call free on hash value. However, as I mention in my post
#4, you could call user defined free function:
if ( _hash->fentry_func != NULL ) _hash->fentry_func(e);
Original comment by mle...@gmail.com
on 5 Feb 2014 at 2:59
yat, that's a nice advice. Thanks...
Original comment by chenxin6...@gmail.com
on 5 Feb 2014 at 10:22
Original issue reported on code.google.com by
mle...@gmail.com
on 28 Jan 2014 at 8:23