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

[Harald Welte]

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

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


Patch Set 1: Code-Review-1

(4 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.  "show rejected-bts"?  "show rejected-oml"? Many things can be rejected at many different levels, so just "reejcted" is a bit too generic.


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 think this warrants a #define, probably next to the osmo_sock_get_remote* which specifies the maximum required buffer size for those kind of buffers.

Also, we generally declare all variables at the top of the block, so it should be declared at the line below 'uint16_t trx_id'.


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.  Or you split the block into a separate function like "rejected_bts_find(site_id, bts_id, ip)" which then has those variables as local variables in the scope of that function.


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. So you culd do something like for (i = llist_count(); i > 25; i--) { }



-- 
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-Comment-Date: Mon, 29 Oct 2018 23:06:48 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181029/4cf20e4f/attachment.htm>