Patch Set 1:

(1 comment)

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

Line 692:               gsm340_rx_sms_submit(sms_report);
> code cosmetics: I see from the old sms_route_mt_sms() that it looks correct
I'm going to try to address all your concerns in one go.

With SMPP support, we're getting this from the SMPP SUBMIT_SM, so there is no 
mti here. In nitb mode, the sms_report object is created from the original sms 
that we get from the mobile station. I don't think the old switch(mti) belong 
sms_route_mt_sms().

I don't see where the name "sms_route_mt_sms()" suggests it has been already 
sent :-), we can probably rename this to simply sms_route() given that no mti 
handling happens anymore.

I agree the log could be moved after the gsms->receiver branch, it would be 
more intuitive, yes. Anyway, this code runs inconditionally, so from the 
logging perspective, this would be printed in the right.

Regarding the gsm->receiver indicator, the SMSC handles both SMPP and nitb 
modes. With SMPP there is no receiver, we really don't know since it's the ESME 
the one routing all SMS. We can probably wrap this in some simple:

static inline bool sms_has_route(struct gsm_sms *sms)
{
       return gsm->receiver;
}

Thanks for your fast review!


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5cc7bb4ebadde0940f44d10c3df34707b0615160
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Holger Freyther <hol...@freyther.de>
Gerrit-Reviewer: Keith Whyte <ke...@rhizomatica.org>
Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Pablo Neira Ayuso <pa...@gnumonks.org>
Gerrit-HasComments: Yes

Reply via email to