{"action":"created","issue":{"url":"https://api.github.com/repos/systemd/systemd/issues/11140","repository_url":"https://api.github.com/repos/systemd/systemd","labels_url":"https://api.github.com/repos/systemd/systemd/issues/11140/labels{/name}","comments_url":"https://api.github.com/repos/systemd/systemd/issues/11140/comments","events_url":"https://api.github.com/repos/systemd/systemd/issues/11140/events","html_url":"https://github.com/systemd/systemd/pull/11140","id":390437493,"node_id":"MDExOlB1bGxSZXF1ZXN0MjM4MjA0Nzc2","number":11140,"title":"Use ?: to chain comparison functions (for now, in lldp only)","user":{"login":"filbranden","id":106037,"node_id":"MDQ6VXNlcjEwNjAzNw==","avatar_url":"https://avatars0.githubusercontent.com/u/106037?v=4","gravatar_id":"","url":"https://api.github.com/users/filbranden","html_url":"https://github.com/filbranden","followers_url":"https://api.github.com/users/filbranden/followers","following_url":"https://api.github.com/users/filbranden/following{/other_user}","gists_url":"https://api.github.com/users/filbranden/gists{/gist_id}","starred_url":"https://api.github.com/users/filbranden/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/filbranden/subscriptions","organizations_url":"https://api.github.com/users/filbranden/orgs","repos_url":"https://api.github.com/users/filbranden/repos","events_url":"https://api.github.com/users/filbranden/events{/privacy}","received_events_url":"https://api.github.com/users/filbranden/received_events","type":"User","site_admin":false},"labels":[{"id":280555991,"node_id":"MDU6TGFiZWwyODA1NTU5OTE=","url":"https://api.github.com/repos/systemd/systemd/labels/lldp","name":"lldp","color":"bfdadc","default":false},{"id":222775092,"node_id":"MDU6TGFiZWwyMjI3NzUwOTI=","url":"https://api.github.com/repos/systemd/systemd/labels/reviewed/needs-rework%20%F0%9F%94%A8","name":"reviewed/needs-rework 🔨","color":"eb6420","default":false},{"id":224437857,"node_id":"MDU6TGFiZWwyMjQ0Mzc4NTc=","url":"https://api.github.com/repos/systemd/systemd/labels/tests","name":"tests","color":"c7def8","default":false},{"id":222868150,"node_id":"MDU6TGFiZWwyMjI4NjgxNTA=","url":"https://api.github.com/repos/systemd/systemd/labels/util-lib","name":"util-lib","color":"fad8c7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2018-12-12T22:31:15Z","updated_at":"2018-12-14T21:34:20Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/systemd/systemd/pulls/11140","html_url":"https://github.com/systemd/systemd/pull/11140","diff_url":"https://github.com/systemd/systemd/pull/11140.diff","patch_url":"https://github.com/systemd/systemd/pull/11140.patch"},"body":"This is in some ways a follow up to #9789 and part of the [discussion](https://github.com/systemd/systemd/pull/9789#discussion_r208072862) we had there.\r\n\r\nIt turns out the `?:` operator in C does just what we want. It keeps the result if non-zero, otherwise keeps evaluating the next calls on the chain.\r\n\r\nI started with this simple PR that just uses it in one place, but if this looks acceptable, I'd like to refactor more of the compare_func's to use this.\r\n\r\nAnother idea is to simplify `memcmp()` + a `CMP()` for the lengths into a single macro that takes two pointers and two lengths (not sure about the name... `BINCMP`? `BSTRCMP`? `BUFCMP`? `MEMCMP`)\r\n\r\nAlso, one for booleans `BOOLCMP()` would make sense, to convert [code like this](https://github.com/systemd/systemd/blob/v239/src/libsystemd/sd-event/sd-event.c#L323):\r\n\r\n```c\r\n        if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)\r\n                return -1;\r\n        if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)\r\n                return 1;\r\n```\r\n\r\ninto:\r\n\r\n```c\r\n      return BOOLCMP(x->enabled != SD_EVENT_OFF, y->enabled != SD_EVENT_OFF)\r\n          ?: CMP(x->priority, y->priority)\r\n          ?: CMP(x->pending_iteration, y->pending_iteration);\r\n```\r\n\r\n(I'm not 100% whether `!=` or `==` is better here, but you get the idea...)\r\n\r\nAnyways, starting with a simple one to get the ball rolling, we can get the next ones done later.\r\n\r\nCheers,\r\nFilipe\r\n"},"comment":{"url":"https://api.github.com/repos/systemd/systemd/issues/comments/447485890","html_url":"https://github.com/systemd/systemd/pull/11140#issuecomment-447485890","issue_url":"https://api.github.com/repos/systemd/systemd/issues/11140","id":447485890,"node_id":"MDEyOklzc3VlQ29tbWVudDQ0NzQ4NTg5MA==","user":{"login":"filbranden","id":106037,"node_id":"MDQ6VXNlcjEwNjAzNw==","avatar_url":"https://avatars0.githubusercontent.com/u/106037?v=4","gravatar_id":"","url":"https://api.github.com/users/filbranden","html_url":"https://github.com/filbranden","followers_url":"https://api.github.com/users/filbranden/followers","following_url":"https://api.github.com/users/filbranden/following{/other_user}","gists_url":"https://api.github.com/users/filbranden/gists{/gist_id}","starred_url":"https://api.github.com/users/filbranden/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/filbranden/subscriptions","organizations_url":"https://api.github.com/users/filbranden/orgs","repos_url":"https://api.github.com/users/filbranden/repos","events_url":"https://api.github.com/users/filbranden/events{/privacy}","received_events_url":"https://api.github.com/users/filbranden/received_events","type":"User","site_admin":false},"created_at":"2018-12-14T21:34:20Z","updated_at":"2018-12-14T21:34:20Z","author_association":"MEMBER","body":"Ok I figured out the memory leak... The problem is that `hashmap_clear()` will call the destructor on each value, the destructor for these LLDP entries is `lldp_neighbor_unlink()` which itself will call `hashmap_remove()` and move things around in the hashmap, therefore leaking some objects...\r\n\r\nI'll update the destructor to use a function that does all but call `hashmap_remove()`, I guess that should fix the leak. I'll send a separate PR for that."}}
{"actor":{"display_login":"filbranden"}}