libosmo-sccp[master]: sccp: enable global addressbook search

[Neels Hofmeyr]

Patch Set 2: Code-Review-1

(2 comments)

As I said in person, my humble opinion goes against keeping a second list. I see less bug surface if we simply iterate the ss7 instances' address lists. When each address is in exactly one list (only with its ss7 instance), we don't need to take care to keep the separate lists in sync in potentially numerous places. (I am tempted to accept the patch, but the more I think about it the less I like it)

https://gerrit.osmocom.org/#/c/3265/2/src/osmo_ss7_vty.c
File src/osmo_ss7_vty.c:

Line 987: 					      *ss7)
(As discussed in person yesterday:)

There's a mutability problem with this function, even before this patch: it is possible to inadvertently change an address book entry. The returned address should be const, or we shoul d copy the found address to a destination struct provided by the caller.

Also, if I search globally, I want to know which ss7 instance the address was found in.

We discussed yesterday that a good function signature would be:

  struct osmo_ss7_instance *osmo_sccp_addr_by_name(const char *name, struct osmo_sccp_addr *dest)

It is then not possible to provide a specific instance to search in, but with the global address namespace we deemed that not necessary / to be added later if needed.

If no address was found, a NULL return value can indicate that.


Line 1000:  *  \param[in] ss7 instance (NULL to search global)
s/global/in all ss7 instances


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5acc1e5abc3b3081149a9f476038e4e53d23b763
Gerrit-PatchSet: 2
Gerrit-Project: libosmo-sccp
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