[Asterisk-code-review] core: Disable astobj2 locking for some common objects. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/10378 )

Change subject: core: Disable astobj2 locking for some common objects.
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/10378/1/include/asterisk/astobj2.h
File include/asterisk/astobj2.h:

https://gerrit.asterisk.org/#/c/10378/1/include/asterisk/astobj2.h@408
PS1, Line 408: #define ao2_alloc_nolock(data_size, destructor_fn) \
             : 	__ao2_alloc((data_size), (destructor_fn), AO2_ALLOC_OPT_LOCK_NOLOCK, "",  __FILE__, __LINE__, __PRETTY_FUNCTION__)
             : #define ao2_t_alloc_nolock(data_size, destructor_fn, tag) \
             : 	__ao2_alloc((data_size), (destructor_fn), AO2_ALLOC_OPT_LOCK_NOLOCK, (tag),  __FILE__, __LINE__, __PRETTY_FUNCTION__)
> The allocator options are so long that it always requires wrapping the arguments. […]
It doesn't reduce readability much especially if the assignment is done outside an if test:

obj = aot_alloc_options(sizeof(*obj), obj_dtor,
   AO2_ALLOC_OPT_LOCK_NOLOCK);
if (!obj) {
}

Besides having a blizzard of aliases for things requires you to be more cognizant of what's available when writing and reviewing.

The reason ao2_alloc_options() exists at all is because of the growth of the API.  I would rather it disappeared to just become ao2_alloc().  Unfortunately, that isn't really possible because of the amount of code that would need changing.  Similar for ao2_container_alloc() and ao2_container_alloc_options() in favor of ao2_container_alloc_list(), ao2_container_alloc_hash(), and ao2_container_alloc_rbtree().

So I'd rather not grow the API for trivial things like this when there are perfectly valid other calls available.



-- 
To view, visit https://gerrit.asterisk.org/10378
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie999b9941760be3d1946cdb6e30cb85fd97504d8
Gerrit-Change-Number: 10378
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2 (1000185)
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Tue, 02 Oct 2018 20:19:37 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20181002/8bfff365/attachment.html>