Change in osmo-bsc[master]: vty: add 'show rejected'

[osmith]

osmith has posted comments on this change. ( https://gerrit.osmocom.org/11493 )

Change subject: vty: add 'show rejected'
......................................................................


Patch Set 1:

(5 comments)

https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bsc_vty.c
File src/osmo-bsc/bsc_vty.c:

https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bsc_vty.c@544
PS1, Line 544: show rejected
> I think in terms of naming this needs to be further qualified. […]
Done


https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bsc_vty.c@562
PS1, Line 562: 		char buf[20];
is this okay since this is in a different scope, or should buf also be declared on top of the function?


https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bts_ipaccess_nanobts.c
File src/osmo-bsc/bts_ipaccess_nanobts.c:

https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bts_ipaccess_nanobts.c@475
PS1, Line 475: 64
> I've seen this magic number appearing two times now. […]
I have copy pasted the 64 from osmo_sock_get_name() without thinking too much about it. Turns out, the right constant to use would be INET6_ADDRSTRLEN (46).

If you want, I can make a patch for libosmocore that uses INET6_ADDRSTRLEN there instead of the 64. Also the right number for the port string length would be 6 (as 65535 is the highest allowed port number). Right now, the code has portbuf[16] - should we introduce a #define for that (there isn't one in POSIX like INET6_ADDRSTRLEN for ports)?

You've introduced both magic numbers here:
https://git.osmocom.org/libosmocore/commit/?id=48f55833476439fc45fa4eaa4327beccdc92d44b


https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bts_ipaccess_nanobts.c@480
PS1, Line 480: struct gsm_bts_rejected *entry = NULL;
             : 	struct gsm_bts_rejected *pos;
> same for those variables, they should be at the top of the block. […]
Done


https://gerrit.osmocom.org/#/c/11493/1/src/osmo-bsc/bts_ipaccess_nanobts.c@506
PS1, Line 506: llist_for_each_entry(pos, &bsc_gsmnet->bts_rejected, list) {
             : 		if (i >= 25)
> we do have llist_count() to count the number of entries. […]
Done



-- 
To view, visit https://gerrit.osmocom.org/11493
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iba3bfe8fc9432b7ae8f819df8bd71b35b3ec507e
Gerrit-Change-Number: 11493
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Comment-Date: Tue, 30 Oct 2018 13:04:24 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181030/ca2f03fe/attachment.htm>