[asterisk-dev] [Code Review] unchecked_return audit and fixes
Tilghman Lesher
reviewboard at asterisk.org
Tue May 8 13:55:44 CDT 2012
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1905/#review6172
-----------------------------------------------------------
/branches/1.8/apps/app_voicemail.c
<https://reviewboard.asterisk.org/r/1905/#comment11310>
Why not just use strerror() here?
/branches/1.8/main/asterisk.c
<https://reviewboard.asterisk.org/r/1905/#comment11309>
Adding this error message here will cause the log to be continuously spammed as fast as the program can run. Remember, this is inside an infinite loop, so if the file is deleted, you're just going to keep pumping out this error message without ever actually handling the condition. Not good.
/branches/1.8/main/pbx.c
<https://reviewboard.asterisk.org/r/1905/#comment11308>
Is it really necessary to put the tool name in a comment? This sort of feels like comments that include someone's initials, which are contrary to coding guidelines.
/branches/1.8/res/ael/ael.flex
<https://reviewboard.asterisk.org/r/1905/#comment11307>
Now that I see what this code does, it'd probably be better if this stat were simply replaced with two lseek()s, instead (which is much faster). The same is true for the earlier case in this file.
- Tilghman
On May 8, 2012, 12:59 p.m., jrose wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1905/
> -----------------------------------------------------------
>
> (Updated May 8, 2012, 12:59 p.m.)
>
>
> Review request for Asterisk Developers and Matt Jordan.
>
>
> Summary
> -------
>
> Makes corrections for a number of unchecked return reports from Coverity and marks a handful of others as being ignorable (because the return seems genuinely inconsequential for these cases). There are a couple oddities that might need a second look.
>
>
> This addresses bug ASTERISK-19658.
> https://issues.asterisk.org/jira/browse/ASTERISK-19658
>
>
> Diffs
> -----
>
> /branches/1.8/apps/app_queue.c 365473
> /branches/1.8/apps/app_voicemail.c 365473
> /branches/1.8/channels/chan_iax2.c 365473
> /branches/1.8/channels/chan_sip.c 365473
> /branches/1.8/channels/iax2-provision.c 365473
> /branches/1.8/channels/sig_analog.c 365473
> /branches/1.8/funcs/func_devstate.c 365473
> /branches/1.8/funcs/func_lock.c 365473
> /branches/1.8/main/acl.c 365473
> /branches/1.8/main/asterisk.c 365473
> /branches/1.8/main/db.c 365473
> /branches/1.8/main/features.c 365473
> /branches/1.8/main/pbx.c 365473
> /branches/1.8/main/xmldoc.c 365473
> /branches/1.8/res/ael/ael.flex 365473
>
> Diff: https://reviewboard.asterisk.org/r/1905/diff
>
>
> Testing
> -------
>
> Testing done depends on the particular issue that was being poked at. In some cases, it was as simple as a snap judgement to add a warning. In a couple others, I would test what would happen as a consequence of an unanticipated return.
>
>
> Thanks,
>
> jrose
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120508/fb99152d/attachment-0001.htm>
More information about the asterisk-dev
mailing list