osmo-bts[master]: octphy: Fix VTY commands

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Tue Jan 31 11:01:15 UTC 2017


Patch Set 1: Code-Review-1

(9 comments)

https://gerrit.osmocom.org/#/c/1711/1//COMMIT_MSG
Commit Message:

Line 7: octphy: Fix VTY commands
summary should mention the rough area, here e.g. "VTY: fix 'show phy'". But this is not really a fix but a first implementation? It's not clear from this patch whether the stats are also printed in the log and printing in the VTY is added on top, or whether the stats are so far not printed anywhere (so please clarify in this log msg).


Line 12: the problem
(put the vty commands in quotes)

(don't say 'fixes' since it is a first implementation?)


https://gerrit.osmocom.org/#/c/1711/1/src/osmo-bts-octphy/octphy_hw_api.c
File src/osmo-bts-octphy/octphy_hw_api.c:

Line 114
mention code moves and the reason (who needs it?) in the commit log for easier review


https://gerrit.osmocom.org/#/c/1711/1/src/osmo-bts-octphy/octphy_vty.c
File src/osmo-bts-octphy/octphy_vty.c:

Line 172: 		(tOCTVC1_HW_MSG_RF_PORT_STATS_RSP *) resp->l2h;
check for correct msgb size?


Line 174: 	vty_out(vty,"%s", VTY_NEWLINE);
(space after comma, ~30 times)


Line 204: 	struct octphy_hw_get_cb_data cb_data;
static struct (see below)


Line 210: 				    &cb_data);
segfault due to lifetime: cb_data becomes deallocated as soon as this function exits, but is needed until the callback fires. Either allocate and deallocate or just make the struct static (collision with multiple calls is not harmful).


Line 220: 		(tOCTVC1_HW_MSG_CLOCK_SYNC_MGR_STATS_RSP *) resp->l2h;
check for correct msgb size?


Line 243: 	struct octphy_hw_get_cb_data cb_data;
static (same reason as above)


-- 
To view, visit https://gerrit.osmocom.org/1711
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae5aa91fe2ebba7c2874eed88b15ed66e8c9cd61
Gerrit-PatchSet: 1
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes


More information about the gerrit-log mailing list