[Asterisk-code-review] channel: Hang-up crashed Asterisk while freeing private data. (asterisk[11])

[Alexander Traud]

Alexander Traud has posted comments on this change.

Change subject: channel: Hang-up crashed Asterisk while freeing private data.
......................................................................


Patch Set 1:

(2 comments)

https://gerrit.asterisk.org/#/c/3731/1/main/channel.c
File main/channel.c:

PS1, Line 2352: 		ast_log_callid(LOG_WARNING, callid, "Channel '%s' may not have been hung up" \
              : 			" properly; leaking memory. Please report via http://issues.asterisk.org." \
              : 			" If feasible, please include" \
              : 			" http://wiki.asterisk.org/wiki/display/AST/Getting+a+Backtrace" \
              : 			" via an interactive session in the GNU Debugger (gdb).\n",
              : 			ast_channel_name(chan));
> This is really long winded.  How about:
Done (partially)

I added the assert, I guess for the Developer Mode. I am not sure, when those Assert are evaluated actually.

When it comes to the error message, I love to see a more concise variant. The reasons why I have chosen the current message:
– the original message should not be changed, so users and developers still find related reports in the Issue Tracker, Mailing Lists, and the web based Forum. That way, duplicates can be found more easily.
– this bug is present for more than 6 years in Asterisk. Although there were a lot of reports, only the causing bug got tackled each time. Furthermore, many reports got closed because the given information were incomplete; stacktraces were missing.
– the memory leak is just a side-effect of the cause issue. The user should not start a memory leak investigation.

Now, with the current change, no stacktrace is created anymore automatically. The user has to do that manually (or go for the Developer Mode thanks to the Assert). The idea of this message is to encourage those who face that issue, raise participation, and giving a helping hand where to start/what to supply.


PS1, Line 2352: 		ast_log_callid(LOG_WARNING, callid, "Channel '%s' may not have been hung up" \
              : 			" properly; leaking memory. Please report via http://issues.asterisk.org." \
              : 			" If feasible, please include" \
              : 			" http://wiki.asterisk.org/wiki/display/AST/Getting+a+Backtrace" \
              : 			" via an interactive session in the GNU Debugger (gdb).\n",
              : 			ast_channel_name(chan));
> This should be changed to LOG_ERROR.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id085417d7e0a5c61382582e7d250b56ab3c00a1b
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes