[asterisk-dev] [Code Review] Bring the (missing) changes from Mantis ID 13495 in trunk.

[wdoekes]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1653/#review5207
-----------------------------------------------------------


I have no clue about the functionality of the SS7 code, but here are a couple of general comments:


trunk/isup.c
<https://reviewboard.asterisk.org/r/1653/#comment9704>

    Here you use hexadecimal ...



trunk/isup.c
<https://reviewboard.asterisk.org/r/1653/#comment9705>

    ... yet here you use decimal. Better choose one or the other.



trunk/isup.c
<https://reviewboard.asterisk.org/r/1653/#comment9706>

    (1) prefer sizeof(*c) over sizeof(struct some_struct). That way changes to the struct name will have to be done in fewer places.
    
    (2) memset is not needed. calloc already zeroes things out.



trunk/mtp3.c
<https://reviewboard.asterisk.org/r/1653/#comment9707>

    Are the parentheses around NET_* needed?



trunk/mtp3.c
<https://reviewboard.asterisk.org/r/1653/#comment9708>

    If this adheres to the asterisk coding guidelines, you'll want to add braces {} here.



trunk/ss7.c
<https://reviewboard.asterisk.org/r/1653/#comment9709>

    Here name[] is only guaranteed to be 1 byte. You're writing anywhere from 5 to 23 bytes in it.



trunk/ss7.c
<https://reviewboard.asterisk.org/r/1653/#comment9710>

    Braces {} again.


- wdoekes


On Jan. 18, 2012, 3:52 a.m., KNK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1653/
> -----------------------------------------------------------
> 
> (Updated Jan. 18, 2012, 3:52 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> libss7 part of the changes only
> 
> 
> Diffs
> -----
> 
>   trunk/isup.h 296 
>   trunk/isup.c 296 
>   trunk/libss7.h 296 
>   trunk/mtp2.h 296 
>   trunk/mtp2.c 296 
>   trunk/mtp3.h 296 
>   trunk/mtp3.c 296 
>   trunk/ss7.c 296 
>   trunk/ss7_internal.h 296 
> 
> Diff: https://reviewboard.asterisk.org/r/1653/diff
> 
> 
> Testing
> -------
> 
> compile test only
> 
> 
> Thanks,
> 
> KNK
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120118/9f2cdede/attachment.htm>