osmo-hlr[master]: rewrite subscriber_update_notify() without calls into luop

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Apr 19 12:38:21 UTC 2018


Patch Set 3: Code-Review+1

(4 comments)

>From laforge's comments, it would suffice to just store a cn_domain per peer. But we would need a check to ensure that we don't overwrite a cn_domain of the peer back and forth. I think these separate flags are an easier way of handling it (by just allowing it). What do you think, laforge?

If a peer exists and is neither indicated as is_cs nor is_ps, then by definition this peer has never done a lu_op and it could be skipped entirely, right?

I don't have a very strong opinion on any of this, except the code dup -- in this instance there could be a trivial function containing the identical parts to silence my bickering?

And that line marked "?" :)

https://gerrit.osmocom.org/#/c/7743/3//COMMIT_MSG
Commit Message:

Line 14: into a common implementation, but that is left for future work.
If you're intending to let it stay like this, I would actually still oppose. If this is just an intermediate step that you will follow up on soon, then I'm fine with it. This paragraph reads like we'll have the code dup for ever -- let's avoid broken windows.


https://gerrit.osmocom.org/#/c/7743/3/src/hlr.c
File src/hlr.c:

Line 64: 	llist_for_each_entry(co, &g_hlr->gs->clients, list) {
cosmetic wise, I'd prefer a separate function that is called from the loop for each peer, separating the action of sending from the decision of who should get it; thinking ahead: changing the code to notify only those peers that a subscriber was last seen at would then not need an edit of that function.


Line 100: 			gsup.cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
?


Line 112: 			gsup.cn_domain = OSMO_GSUP_CN_DOMAIN_CS;
I think I'd

  if (!co->supports_ps && !co->supports_cs)
    continue;

above


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I06c43ece2b48dc63d599000eb6d6d51e08963067
Gerrit-PatchSet: 3
Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list