{"action":"created","issue":{"url":"https://api.github.com/repos/xant/libhl/issues/8","labels_url":"https://api.github.com/repos/xant/libhl/issues/8/labels{/name}","comments_url":"https://api.github.com/repos/xant/libhl/issues/8/comments","events_url":"https://api.github.com/repos/xant/libhl/issues/8/events","html_url":"https://github.com/xant/libhl/issues/8","id":103426753,"number":8,"title":"Issues","user":{"login":"graphitemaster","id":343195,"avatar_url":"https://avatars.githubusercontent.com/u/343195?v=3","gravatar_id":"","url":"https://api.github.com/users/graphitemaster","html_url":"https://github.com/graphitemaster","followers_url":"https://api.github.com/users/graphitemaster/followers","following_url":"https://api.github.com/users/graphitemaster/following{/other_user}","gists_url":"https://api.github.com/users/graphitemaster/gists{/gist_id}","starred_url":"https://api.github.com/users/graphitemaster/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/graphitemaster/subscriptions","organizations_url":"https://api.github.com/users/graphitemaster/orgs","repos_url":"https://api.github.com/users/graphitemaster/repos","events_url":"https://api.github.com/users/graphitemaster/events{/privacy}","received_events_url":"https://api.github.com/users/graphitemaster/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-08-27T04:46:05Z","updated_at":"2015-08-27T13:25:16Z","closed_at":null,"body":"Some quick initial review of libhl turned up some issues with it.\r\n\r\n * The use of ```__USE_UNIX98``` is a non-standard feature-test macro which should be replaced with ```_POSIX_C_SOURCE```.\r\n\r\n* 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.\r\n\r\n* 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.\r\n\r\n* ```#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.)\r\n\r\n* 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.\r\n\r\n* 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.\r\n\r\n* Failure to check return value of ```malloc```. It can fail, without checking and handling this case, this library is not usable for robust systems."},"comment":{"url":"https://api.github.com/repos/xant/libhl/issues/comments/135427044","html_url":"https://github.com/xant/libhl/issues/8#issuecomment-135427044","issue_url":"https://api.github.com/repos/xant/libhl/issues/8","id":135427044,"user":{"login":"elfring","id":660477,"avatar_url":"https://avatars.githubusercontent.com/u/660477?v=3","gravatar_id":"","url":"https://api.github.com/users/elfring","html_url":"https://github.com/elfring","followers_url":"https://api.github.com/users/elfring/followers","following_url":"https://api.github.com/users/elfring/following{/other_user}","gists_url":"https://api.github.com/users/elfring/gists{/gist_id}","starred_url":"https://api.github.com/users/elfring/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/elfring/subscriptions","organizations_url":"https://api.github.com/users/elfring/orgs","repos_url":"https://api.github.com/users/elfring/repos","events_url":"https://api.github.com/users/elfring/events{/privacy}","received_events_url":"https://api.github.com/users/elfring/received_events","type":"User","site_admin":false},"created_at":"2015-08-27T13:25:16Z","updated_at":"2015-08-27T13:25:16Z","body":"* I suggest to avoid [ignorance of return values](https://cwe.mitre.org/data/definitions/252.html \"Unchecked return values\") a bit more.\r\n\r\n  Are you interested to apply [aspect-oriented software development](http://aspectc.org/ \"AspectC++\")?\r\n  How do you think about to encapsulate error detection and corresponding exception handling as [a reusable aspect](http://en.wikipedia.org/wiki/Aspect_%28computer_programming%29 \"Combination of a pointcut with advice\") in your software?\r\n* How do you think about to avoid that this software [depends on undefined behaviour](https://www.securecoding.cert.org/confluence/display/c/CC.+Undefined+Behavior#CC.UndefinedBehavior-ub_106 \"Avoid undefined behaviour?\")?"}}