Change in libosmo-sccp[master]: ss7: Introduce APIs to manage asp_peer hosts

[osmith]

osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/16795 )

Change subject: ss7: Introduce APIs to manage asp_peer hosts
......................................................................


Patch Set 1:

(8 comments)

https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1//COMMIT_MSG 
Commit Message:

https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1//COMMIT_MSG@9 
PS1, Line 9: coe
(code)


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/include/osmocom/sigtran/osmo_ss7.h 
File include/osmocom/sigtran/osmo_ss7.h:

https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/include/osmocom/sigtran/osmo_ss7.h@432 
PS1, Line 432: local_hosts
(How about "hosts" instead of "local_hosts" and "host_cnt" instead of "local_host_cnt" (as in osmo_ss7.c)? (same in line below))


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/include/osmocom/sigtran/osmo_ss7.h@432 
PS1, Line 432: int osmo_ss7_asp_peer_set_hosts(struct osmo_ss7_asp_peer *peer, void *talloc_ctx, const char* const* local_hosts, size_t local_host_cnt);
(line is > 100 characters)


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c 
File src/osmo_ss7.c:

https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@1116 
PS1, Line 1116: int osmo_ss7_asp_peer_set_hosts(struct osmo_ss7_asp_peer *peer, void *talloc_ctx, const char* const* hosts, size_t host_cnt)
(line > 100 characters)


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@1135 
PS1, Line 1135: 
(empty line should not be there)


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@1141 
PS1, Line 1141: 	/* Makes no sense to have INET_ANY and specific addresses in the set */
              : 	for (i = 0; i < peer->host_cnt; i++) {
              : 			iter_is_any = !peer->host[i] ||
              : 				      !strcmp(peer->host[i], "0.0.0.0");
              : 			if (new_is_any && iter_is_any)
              : 				return -EINVAL;
              : 			if (!new_is_any && iter_is_any)
              : 				return -EINVAL;
              : 	}
              : 	/* Makes no sense to have INET_ANY many times */
              : 	if (new_is_any && peer->host_cnt)
              : 		return -EINVAL;
(The logic here seems flawed / can be simplified (e.g. move the if (new_is_any ... check up, and we may not even need the for loop if there can only be one INET_ANY address in peer?)... but as I'm writing this, I realize that you just moved the code. So it's fine to leave it as-is in this patch.)


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@2036 
PS1, Line 2036: rc
This changes the logic; in the previous version, "osmo_stream_srv_link_set_addrs" would not get executed if the ARRAY_SIZE check failed. Is this on purpose, and if so, why not in a separate commit?


https://gerrit.osmocom.org/c/libosmo-sccp/+/16795/1/src/osmo_ss7.c@2046 
PS1, Line 2046: rc
same here



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

Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I4af2a6915ac57c7baa67343bd9414c65154d67f7
Gerrit-Change-Number: 16795
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Mon, 13 Jan 2020 10:38:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200113/3d91549b/attachment.htm>