[Asterisk-code-review] lock: Add named lock capability (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: lock:  Add named lock capability
......................................................................


Patch Set 7: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/2521/7/include/asterisk/_private.h
File include/asterisk/_private.h:

Line 41: int ast_named_locks_init(void);		/*!< Provided by named_locks.c */
There should be a shutdown function as well to release the locks container.


https://gerrit.asterisk.org/#/c/2521/7/include/asterisk/named_locks.h
File include/asterisk/named_locks.h:

PS7, Line 58:  * We can't reference ao2_alloc_opts here so these must be kept synchronized
            :  * with ao2_alloc_opts lock types.
This note is useless as you are referencing the ao2 names the note says you cannot access. :)


https://gerrit.asterisk.org/#/c/2521/7/main/asterisk.c
File main/asterisk.c:

Line 4395: 		printf("Failed: ast_lock_init\n%s", term_quit());
s/ast_lock_init/ast_named_locks_init/


https://gerrit.asterisk.org/#/c/2521/7/tests/test_named_lock.c
File tests/test_named_lock.c:

Line 44: 		(const char *)data);
The cast shouldn't be necessary.


PS7, Line 112: 	if (lock1 != NULL) {
             : 		ao2_unlock(lock1);
             : 	}
             : 
             : 	if (lock2 != NULL) {
             : 		ao2_unlock(lock2);
             : 	}
These unlocks should be done right after locking them.  Unlocking without first getting the lock is an error.


PS7, Line 123: 	pthread_kill(thread1, SIGURG);
             : 	pthread_kill(thread2, SIGURG);
You need to
pthread_join(thread1, NULL);
pthread_join(thread2, NULL);
the threads or you will leak the thread memory since you created them joinable.

You shouldn't need to pthread_kill them because they should die in 3 seconds and the join will wait for them to die.


-- 
To view, visit https://gerrit.asterisk.org/2521
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: If258c0b7f92b02d07243ce70e535821a1ea7fb45
Gerrit-PatchSet: 7
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes