openbsc[master]: libmsc: add support for SMPP delivery receipts

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/.

Pablo Neira Ayuso gerrit-no-reply at lists.osmocom.org
Tue Aug 8 11:25:01 UTC 2017


Patch Set 1:

(5 comments)

https://gerrit.osmocom.org/#/c/3434/1/openbsc/src/libmsc/gsm_04_11.c
File openbsc/src/libmsc/gsm_04_11.c:

Line 637: 	if (!sms_report)
> log error, or maybe OSMO_ASSERT(sms_report)?
Is there anything the user can do after this error?

Why bother?

I mean, you only hit this is there is not memory left... So likely there is a leak.

OSMO_ASSERT() would stop openBSC from running, is that getting this any better? This sounds like calling BUG() in the kernel.

So what do we get from spamming the user with an error that *only* happens if we have no memory left?


https://gerrit.osmocom.org/#/c/3434/1/openbsc/src/libmsc/smpp_openbsc.c
File openbsc/src/libmsc/smpp_openbsc.c:

Line 521: 	/* We got a DELIVER_SM response for this is status report, this was
> (hard to understand, fix typo/punctuation?)
This is a reply to a DELIVER_SM with esm_class = Delivery Receipt, we're just having a conversation that was initiated by the SMSC. So we don't need to send any RP-ACK to the mobile, as we do with DELIVER_SM for a plan SMS.


Line 659: 		deliver.esm_class = 0x04;
> (would be nice to have constants instead of magic numbers ... but not relat
I agree, this applies all over the place, but that would require a bit of work.


Line 709: 		       sms->msg_ref);
> user message reference was always missing? maybe the fact that it is added 
Yes, message reference was always missing because there was no status-reports ;-).

We can just add a branch here to skip this if saving bytes on the network are really a concern at the cost of slowing down the CPU path.


https://gerrit.osmocom.org/#/c/3434/1/openbsc/src/libmsc/smpp_smsc.h
File openbsc/src/libmsc/smpp_smsc.h:

Line 95: 	bool			report;
> (can be understood as "you should report" or "this is a report" ... would b
This just annotates that the SMPP command that we send represents a deliver-report.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1a9023074bfa938099377980b6aff9b262fab2a
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pablo at gnumonks.org>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Holger Freyther <holger at freyther.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Keith Whyte <keith at rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pablo at gnumonks.org>
Gerrit-HasComments: Yes



More information about the gerrit-log mailing list