Change in libosmo-abis[master]: Add vty show function for e1d driver

[pespin]

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/24197 )

Change subject: Add vty show function for e1d driver
......................................................................


Patch Set 1:

(6 comments)

https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c 
File src/input/e1d.c:

https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c@326 
PS1, Line 326: 	g_e1d = osmo_e1dp_client_create(NULL, "/tmp/osmo-e1d.ctl");
Hint: you can do first "if (g_e1d) return 0;" here, and then you always simply call the function without having to do the check. then you keep all the g_e1d allocation logic here in this function.


https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c@479 
PS1, Line 479: const struct value_string osmo_e1dp_ts_mode_names[] = {
> I think this can be exposed in osmo-e1d, but I'm not sure exactly […]
We usually tend to add a public function returning a pointer to the start of the array or even resolving the name base don the value, grep for "value_string" in libosmocore and you'll see lots of examples.

For array "osmo_e1dp_ts_mode_names", the function would be called "osmo_e1dp_ts_mode_name" iirc.


https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c@496 
PS1, Line 496: 		if (e1d_daemon_connect(line))
In general I prefer checkjing explicitly against "< 0" if the function returns a negative error on error, it's clearer when reading it.


https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c@507 
PS1, Line 507: 	for (ts=1; ts<line->num_ts; ts++) {
spacing ts = 1


https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c@508 
PS1, Line 508: 		struct e1inp_ts *e1i_ts = &line->ts[ts-1];
spacing ts -1


https://gerrit.osmocom.org/c/libosmo-abis/+/24197/1/src/input/e1d.c@512 
PS1, Line 512: 			bfd->fd, VTY_NEWLINE);
why are you starting with ts=1, then use ts-1, then here ts? it looks wrong.



-- 
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/24197
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I287c82b2c9e4b9b9b30a302e5240d5688b93240c
Gerrit-Change-Number: 24197
Gerrit-PatchSet: 1
Gerrit-Owner: keith <keith at rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Tue, 11 May 2021 09:31:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: keith <keith at rhizomatica.org>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210511/84af3a0f/attachment.htm>