xant / libhl Goto Github PK

I suggest to reuse a higher level build system than your current small make file so that powerful checks for software features will become easier.

• CMake
• Autotools

Would you like to add the configuration script "AX_PTHREAD" to your build specification?

1. Would you like to add more error handling for return values from functions like the following?
   • pthread_mutex_lock ⇒ list_sort
   • pthread_spin_init ⇒ ht_set_list
2. How do you think about to annotate any functions with "__attribute__((warn_unused_result))"?

Currently the tagged value structure for linked lists assumes that the length of a value can fit inside uint32_t. While I'd agree that value types larger than 0xffffffff bytes are pathological; use of size_t instead is of no harm and does not blindly break the interface contract of strlen and would allow you to remove the cast on this line in particular newval->vlen = (uint32_t)strlen((char *)val);.

When representing the size of something it's always best to use size_t which is specifically guaranteed to be large enough to hold the size of any represent-able allocation or object in memory. This is why allocation functions expect size_t, why strlen returns it and why the sizeof operator does as well.

I'm currently unable to share the code that triggers this memory leak but I have traced it using valgrind through the following calls:

==77605== 4,811,032 (4,066,576 direct, 744,456 indirect) bytes in 187,682 blocks are definitely lost in loss record 10 of 10
==77605==    at 0x483DFAF: realloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==77605==    by 0x10E6F3: binomial_tree_merge (binheap.c:399)
==77605==    by 0x10E8B3: binheap_insert (binheap.c:427)
==77605==    by 0x10D12E: pqueue_insert (pqueue.c:78)

This is one of ten leaks that valgrind detects over a relatively short span of inserting items into a priority queue. It seems like the realloc from binomial_tree_merge is not beeing freed.

I am doing something funky in that I am passing size_t values and casting them as void* types in my priority queue but this seems to work. I don't know if there is an underlying assumption anywhere that the values must be pointers (for example, this approach doesn't work with hastable.c because the hash function accesses the pointers; I don't believe pqueue or binheap do this, though).

The number of successfully read from rqueue sometimes less than the number of successfully write to rqueue.

Here is the test code.
https://gist.github.com/mingpepe/9d6ec5f17bdb60094d7fc980df3437ce

But i am not found the bug yet.

Do I need to free key/value after call the ht_delete function?

Three callback functions are assigned to attributes of the variable "keys_callbacks_default".
I find the usage of the specifier "inline" inappropriate for them because a callable address will be needed in this use case.

Hello, I am looking for a hashtable which can hold millions or even more key-value pairs. Can this hashtable hold such payload, and give a good performance when there is some rebalance?

ht_foreach_pair when ,ht_pair_iterator_callback_t fun return value < 0, the hash count not decrement 1

Hi! I written a library. It is open source. I included some files from your library. What should I do now(including your license etc.)?

The macro "AC_INIT" supports that the needed version information is determined by specific macro calls before.
Would you like to reuse anything from the current autoconf approach for your build specification?

I would like to point out that identifiers like "__tagged_value" and "_HL_HASHTABLE_H_" do eventually not fit to the expected naming convention of the C language standard.
Would you like to adjust your selection for unique names?

Use only rbuf_create/rbuf_read/rbuf_write .
Is rbuf lock-free for single-thread read and single-thread write ?
Do I need to use mutex to lock read/write access to rfx/wfx/used?

hi, author. I use create 5 thread to pop queue's arg, there are 10000 args that push to queue. But sometimes, the process will be crash. And from the crash log, i see that queue will call refcnt free and then process crash.

Some quick initial review of libhl turned up some issues with it.

• The use of __USE_UNIX98 is a non-standard feature-test macro which should be replaced with _POSIX_C_SOURCE.
• There are a lot of identifiers that begin with double underscore, for instance __foo. These identifiers are reserved by the implementation (compiler and libc.) perhaps using an hl_ prefix would be better. This is also seen in headers a lot too; __BINOMIAL_HEAP_H__ for instance.
• Failure to use extern "C" in headers for __cplusplus This makes it hard to actually use the library for C++ as the symbol names can be mangled, thus not finding them at link stage.
• #pragma pack(push, ...) is seen a lot. I'm not sure why you're packing these structures, for doing so can only lead to access violations on systems where the read/write may be unaligned and therefor illegal (PPC for instance.)
• Checking for NULL when calling free. This is just redundant as free is required to do this check. Doing this only leads to false-positives when running the code through static-analysis tools like coverity.
• Linked list is not thread-safe. get_entry_position needs to hold a lock for a new list node can be inserted into the list in another thread causing the position to be wrong. This can cause a whole slew of problems.
• Failure to check return value of malloc. It can fail, without checking and handling this case, this library is not usable for robust systems.

How do you think about to benefit from parallel build trees also for your software?
Would you like to support out-of-source (VPATH) builds?

After actual testing, when in the single producer single consumer model, there is an infinite loop problem!

About how to use the opened APIs and the meaning of the return value.

Use hash function on ARM platform, but it crash when callinig ht_create

I suggest to [reuse the software "libtool"](https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Libtool.htm Libtool usage").
Would you like to add the call of the macro "LT_INIT"" to your build specification?

I suggest to add the call of the macro "AC_CONFIG_AUX_DIR" to your configuration script so that auxiliary build tools will be stored in a dedicated directory.
Would such fine-tuning help a bit for the issue "Addition of a build system generator"?