Harald Welte has posted comments on this change. ( https://gerrit.osmocom.org/13608 )
Change subject: ggsn: Add minimalistic PAP support ...................................................................... Patch Set 3: (5 comments) https://gerrit.osmocom.org/#/c/13608/3//COMMIT_MSG Commit Message: https://gerrit.osmocom.org/#/c/13608/3//COMMIT_MSG@12 PS3, Line 12: all, without actually checking any credentials database. > So the MS is expected to send inside PAP the values you configure your APN > with? (the User+Password […] yes, if you add username/password, the MS/UE should send those values in PAP. https://gerrit.osmocom.org/#/c/13608/3/ggsn/ggsn.c File ggsn/ggsn.c: https://gerrit.osmocom.org/#/c/13608/3/ggsn/ggsn.c@518 PS3, Line 518: unsigned int pap_welcome_len = strlen(pap_welcome); > "ARRAY_SIZE(pap_welcome) -1" would make sense too :) I think gcc is smart enough to do the strlen at compile-time these days. It's much more readable, for sure. https://gerrit.osmocom.org/#/c/13608/3/ggsn/ggsn.c@519 PS3, Line 519: uint8_t pap_out_size = sizeof(struct pap_element) + 1 + pap_welcome_len; > Is that +1 for the NULL chat of pap_welcome? please add comment (and add +1 > to the end). the one bytes is for the *length* byte. There is no requirement for NUL termination https://gerrit.osmocom.org/#/c/13608/3/ggsn/ggsn.c@520 PS3, Line 520: struct pap_element *pap_out = alloca(pap_out_size); > nice, didn't know about alloca() you can also use runtime-computed size for arrays these days. So something like having the length passed in as a function argument and then putting "char foo[len_arg];" on the stack works. However, in this case we don't want to allocate an array but 'struct pcap_element' with some extra bytes at the end, and hence I went for alloca. https://gerrit.osmocom.org/#/c/13608/3/ggsn/ggsn.c@526 PS3, Line 526: if (htons(pap_in->len) > pco_in->length) > is htons() fine alignment-access wise? […] ACK, will update. -- To view, visit https://gerrit.osmocom.org/13608 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I81875f30f9f1497199253497f84718510747f731 Gerrit-Change-Number: 13608 Gerrit-PatchSet: 3 Gerrit-Owner: Harald Welte <[email protected]> Gerrit-Reviewer: Harald Welte <[email protected]> Gerrit-Reviewer: Jenkins Builder (1000002) Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Comment-Date: Fri, 12 Apr 2019 10:21:45 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No
