Patch Set 1:

(6 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)?


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?)


Line 552:       if (cmd->report)
add comment to explain why?  (ah, same as above, right)


Line 659:               deliver.esm_class = 0x04;
(would be nice to have constants instead of magic numbers ... but not related 
to this patch)


Line 709:                      sms->msg_ref);
user message reference was always missing? maybe the fact that it is added 
unconditionally deserves a mention in the log msg, or even a separate patch?


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 be 
nice to clarify the name)


-- 
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 <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-HasComments: Yes

Reply via email to