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

[KNK]

> On Jan. 18, 2012, 4:54 a.m., wdoekes wrote:
> > trunk/isup.c, line 307
> > <https://reviewboard.asterisk.org/r/1653/diff/5/?file=23406#file23406line307>
> >
> >     ... yet here you use decimal. Better choose one or the other.

changed all to hex


> On Jan. 18, 2012, 4:54 a.m., wdoekes wrote:
> > trunk/isup.c, lines 2543-2547
> > <https://reviewboard.asterisk.org/r/1653/diff/5/?file=23406#file23406line2543>
> >
> >     (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.

changed to sizeof(*c) and memset removed


> On Jan. 18, 2012, 4:54 a.m., wdoekes wrote:
> > trunk/mtp3.c, line 1097
> > <https://reviewboard.asterisk.org/r/1653/diff/5/?file=23411#file23411line1097>
> >
> >     Are the parentheses around NET_* needed?

Yes, as it is defined as 0x07 | 0x10 (so are all other NET_MNG_XXX messages)


> On Jan. 18, 2012, 4:54 a.m., wdoekes wrote:
> > trunk/mtp3.c, lines 1581-1583
> > <https://reviewboard.asterisk.org/r/1653/diff/5/?file=23411#file23411line1581>
> >
> >     If this adheres to the asterisk coding guidelines, you'll want to add braces {} here.

fixed and also removed the else completely.


> On Jan. 18, 2012, 4:54 a.m., wdoekes wrote:
> > trunk/ss7.c, lines 321-323
> > <https://reviewboard.asterisk.org/r/1653/diff/5/?file=23412#file23412line321>
> >
> >     Here name[] is only guaranteed to be 1 byte. You're writing anywhere from 5 to 23 bytes in it.

some how missed the MTP2_MAX_NAME - fixed


> On Jan. 18, 2012, 4:54 a.m., wdoekes wrote:
> > trunk/ss7.c, lines 537-538
> > <https://reviewboard.asterisk.org/r/1653/diff/5/?file=23412#file23412line537>
> >
> >     Braces {} again.

There are too many places in libss7 with missing braces - tried to fix as much as possible.


- KNK


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


On Jan. 19, 2012, 5:21 a.m., KNK wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1653/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2012, 5:21 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> libss7 part of the changes only
> 
> 
> This addresses bugs SS7-27, SS7-52 and SS7-53.
>     https://issues.asterisk.org/jira/browse/SS7-27
>     https://issues.asterisk.org/jira/browse/SS7-52
>     https://issues.asterisk.org/jira/browse/SS7-53
> 
> 
> Diffs
> -----
> 
>   trunk/ss7.c 296 
>   trunk/libss7.h 296 
>   trunk/mtp2.h 296 
>   trunk/mtp2.c 296 
>   trunk/mtp3.h 296 
>   trunk/mtp3.c 296 
>   trunk/isup.h 296 
>   trunk/isup.c 296 
>   trunk/ss7_internal.h 296 
> 
> Diff: https://reviewboard.asterisk.org/r/1653/diff
> 
> 
> Testing
> -------
> 
> compiles, link setup, cli commands, bassic calls
> 
> 
> Thanks,
> 
> KNK
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120214/72bc6615/attachment.htm>