[Asterisk-code-review] ARI: Creating log channels (asterisk[13])

[Ashley Sanders]

Ashley Sanders has posted comments on this change.

Change subject: ARI: Creating log channels
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/1042/2/main/logger.c
File main/logger.c:

Line 1057: int ast_logger_create_channel(const char *channel, const char *components)
To make it cleaner, I think the return values should be defined in an enum value or in a constant, since there are more values than just -1 and 0 being returned from this function.


Line 1087: return 0
Returning here means that you have to add a second call to unlock the list. I am fine with that mode of thinking - except that you should do it on line 1076 as well and just return 1 there (after also unlocking the list).

However, if you are going for the single entry/single exit  approach, you would assign 0 to success on line 1087 and remove line 1086.

Personally, I am more for the 'get the heck out of this function as early as possible' school of thought than structured programming... I think there are less chances for messing up the intended output of a function if the number of logic paths to get to the function exit are reduced. It just means that you have to do cleanup at each exit location.

It's your call, but I opt for consistency with whichever approach you choose. 

https://en.wikipedia.org/wiki/Structured_programming#Early_exit


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a20e5c75716dfbb6b62fd3474faf55be20bd782
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Scott Emidy <jemidy at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-HasComments: Yes