lathiat
Properly destroy the pthread_mutexattr_t object after mutex initialization
I think the if part in https://github.com/avahi/avahi/blob/4be56a83de4304f723484a8b6c1c94abc598cfc1/.github/workflows/smoke-tests.sh#L210-L212 should be removed to start running it under Valgrind.
Can you expand on why it fails under Valgrind on FreeBSD only?
(Generally the compat libraries aren't actively maintained so I'm not sure whether it should be touched or not at this point. I opened that issue to somewhat document why the test is skipped on FreeBSD under Valgrind)
I'll go ahead and add the "needs-discussion" label because I don't think it should be touched so as not give the impression that it's actually actively maintained. If you are interested in this compat library I'd start with addressing the ThreadSanitizer errors like
==================
WARNING: ThreadSanitizer: data race (pid=385320)
Write of size 4 at 0x724000000020 by main thread:
#0 sdref_new /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:409:27 (null-test+0x4ba497) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#1 DNSServiceRegister /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:1128:19 (null-test+0x4bbe7b) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#2 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:48:5 (null-test+0x4be786) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
Previous write of size 4 at 0x724000000020 by thread T1:
#0 thread_func /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:313:27 (null-test+0x4be101) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
Location is heap block of size 248 at 0x724000000000 allocated by main thread:
#0 calloc <null> (null-test+0x4336fb) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#1 xcalloc /home/vagrant/avahi/avahi-common/malloc.c:95:15 (libavahi-common.so.3+0xcfd) (BuildId: 2fb2b0799bbba3c5309c6e930f45cb957e3590dc)
#2 avahi_malloc0 /home/vagrant/avahi/avahi-common/malloc.c:120:16 (libavahi-common.so.3+0xb94) (BuildId: 2fb2b0799bbba3c5309c6e930f45cb957e3590dc)
#3 avahi_new0_internal /home/vagrant/avahi/avahi-compat-libdns_sd/../avahi-common/malloc.h:59:12 (null-test+0x4bdf2e) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#4 sdref_new /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:369:19 (null-test+0x4ba0ee) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#5 DNSServiceRegister /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:1128:19 (null-test+0x4bbe7b) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#6 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:48:5 (null-test+0x4be786) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
Thread T1 (tid=385336, running) created by main thread at:
#0 pthread_create <null> (null-test+0x43531e) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#1 sdref_new /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:406:9 (null-test+0x4ba47c) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#2 DNSServiceRegister /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:1128:19 (null-test+0x4bbe7b) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
#3 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:48:5 (null-test+0x4be786) (BuildId: 7b4cdf63997e83a5295f029c97dd1f2890aa016e)
SUMMARY: ThreadSanitizer: data race /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:409:27 in sdref_new
==================
Then again I'd just leave it be.
I don't see how these simple fixes would hurt even if this component is obsolete. I opened #783 that should fix the tsan report too.
I'm not saying they would hurt. It's just that personally I'm not particularly interested in reviewing/merging PRs related to those libs. If it's decided that they aren't abandoned the CI should actually start testing them (for example I think https://github.com/avahi/avahi/pull/783 should go along with turning on TSan to make sure that it works and prevent regressions along the way). Either way I'll leave it to someone else to review/merge these PRs.
It still fails under TSan by the way
==================
WARNING: ThreadSanitizer: data race (pid=421068)
Read of size 8 at 0x724000000318 by main thread:
#0 sdref_free /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:426:9 (null-test+0x4bdc33) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#1 sdref_unref /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:467:9 (null-test+0x4b97b9) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#2 DNSServiceRefDeallocate /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:526:9 (null-test+0x4b9801) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#3 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:68:5 (null-test+0x4beae4) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
Previous write of size 8 at 0x724000000318 by thread T4:
#0 thread_func /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:312:19 (null-test+0x4be0db) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
As if synchronized via sleep:
#0 sleep <null> (null-test+0x431d96) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#1 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:63:5 (null-test+0x4bea9c) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
Location is heap block of size 248 at 0x724000000300 allocated by main thread:
#0 calloc <null> (null-test+0x4336fb) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#1 xcalloc /home/vagrant/avahi/avahi-common/malloc.c:95:15 (libavahi-common.so.3+0xcfd) (BuildId: 2fb2b0799bbba3c5309c6e930f45cb957e3590dc)
#2 avahi_malloc0 /home/vagrant/avahi/avahi-common/malloc.c:120:16 (libavahi-common.so.3+0xb94) (BuildId: 2fb2b0799bbba3c5309c6e930f45cb957e3590dc)
#3 avahi_new0_internal /home/vagrant/avahi/avahi-compat-libdns_sd/../avahi-common/malloc.h:59:12 (null-test+0x4bdf2e) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#4 sdref_new /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:368:19 (null-test+0x4ba0ee) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#5 DNSServiceBrowse /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:635:19 (null-test+0x4b996e) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#6 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:61:5 (null-test+0x4bea92) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
Thread T4 (tid=421086, finished) created by main thread at:
#0 pthread_create <null> (null-test+0x43531e) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#1 sdref_new /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:405:9 (null-test+0x4ba47c) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#2 DNSServiceBrowse /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:635:19 (null-test+0x4b996e) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
#3 main /home/vagrant/avahi/avahi-compat-libdns_sd/null-test.c:61:5 (null-test+0x4bea92) (BuildId: 38bda71bab59576f4721970f13e4bed715cf979a)
SUMMARY: ThreadSanitizer: data race /home/vagrant/avahi/avahi-compat-libdns_sd/compat.c:426:9 in sdref_free
==================
ThreadSanitizer: reported 1 warnings
The change itself seems reasonable to me. I never investigated libdns_sd library and how it works. Do not even know example where it is used. But only from looking at the change, it should not make it worse than it already is.
I wonder why it can't be declared deprecated and unmaintained then to help places where it's actually used to act on that (and either drop it or start maintaining it properly)? What currently happens is just whack-a-mole (and it isn't even actually tested by the CI yet).