liuis / leveldb

Automatically exported from code.google.com/p/leveldb
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

No corresponding buffer free function for C API leveldb_get #117

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1.
2.
3.

What is the expected output? What do you see instead?

There should be a free function provided by the library. When leveldb is build 
as a shared library, the consumer of this library could be built by any other 
compile with its own free() function.

I would like to suggest that a new API "void leveldb_free(char* buffer)" be 
added.

What version of the product are you using? On what operating system?

1.5.0 on all platforms

Please provide any additional information below.

Original issue reported on code.google.com by albert....@gmail.com on 18 Sep 2012 at 1:21

GoogleCodeExporter commented 9 years ago
Note that this free function should also be used to free the error info 
returned from leveldb_open()/leveldb_destroy_db() and the return from 
leveldb_property_value().

Original comment by albert....@gmail.com on 18 Sep 2012 at 2:38

GoogleCodeExporter commented 9 years ago
Sorry, but I am not inclined to widen the API for such an obscure use case.

Note that you should be able to deal with this by changing your link order so 
that leveldb code uses the same malloc() routine that corresponds to your 
free().  In fact, I am confused as to how you could reasonably expect to work 
in a situation where there are multiple active memory allocators.  For example, 
what happens if either your code or leveldb code calls strdup().  That will go 
into libc and use whatever malloc() libc ends up resolving to.  But the result 
will be passed to your code which will end up calling your custom free().

Original comment by san...@google.com on 18 Sep 2012 at 3:55

GoogleCodeExporter commented 9 years ago
Just FYI: this is very linux-centric attitude.

The problem described by Albert is very common on Windows, where it's customary 
to build a DLL with compiler X and that DLL will be used by code compiled with 
compiler Y. That's why I had to write leveldb_free() in my Windows port 
(https://code.google.com/r/kkowalczyk-leveldb/source/browse) - I've seen this 
problem many times in the past and I know that it has to be properly addressed 
on Windows.

By not adding this you're effectively saying: on Windows, it's not possible to 
provide binary distribution of Leveldb as a DLL.

Esoteric case? Consider hypothetical leveldb Python bindings that someone might 
want to provide for Windows Python. They couldn't.

The only way to do that by a 3rd party is to provide leveldb as a DLL and 
necessary Python wrapper code. 3rd party cannot know (nor should they) what 
compiler was used to build Python. Not to mention that python.org folks might 
use a different compiler than ActiveState folks and version 2.7 might be built 
with a different compiler than 3.2.

Or C# bindings - again, not possible.

As to your hypothetical strdup() scenario: you can't do it (if you want to 
support leveldb as a Windows DLL). The memory allocated by leveldb has to stay 
within leveldb code or, if it escapes to the user code, a function must be 
provided to free it inside leveldb code (hence the need for leveldb_free).

Hope you'll reconsider.

Original comment by kkowalczyk@gmail.com on 18 Sep 2012 at 6:58

GoogleCodeExporter commented 9 years ago
Ok. Will add "void leveldb_free(void*)" to the C binding.

Original comment by san...@google.com on 8 Oct 2012 at 8:56

GoogleCodeExporter commented 9 years ago
Fixed in 1.6.0.  Added leveldb_free(void*) function to C binding.

Original comment by san...@google.com on 12 Oct 2012 at 7:57