Attention is currently required from: jolly.

dexter has posted comments on this change by jolly. ( 
https://gerrit.osmocom.org/c/onomondo-ipa/+/43031?usp=email )

Change subject: V1.2: Add setDefaultDpAddress function
......................................................................


Patch Set 1: Code-Review+1

(2 comments)

Patchset:

PS1:
I think this patch is correct. As mentioned, I would add a bit more "service 
logic" into the es10c function itself.


File src/ipa/libipa/es10c_set_def_dp_addr.h:

https://gerrit.osmocom.org/c/onomondo-ipa/+/43031/comment/9ea8d8d3_3e414872?usp=email
 :
PS1, Line 20:
This is correct. However when the input and output parameters are simple, I 
sometimes use those ASN.1 generateds structs internally and I fall back to 
req/res structs that contain simplified members.

In es10b_get_euicc_chlg.h you find a radical example that would fit here also, 
you could just define the function like this:

int ipa_es10c_set_def_dp_addr(struct ipa_context *ctx, uint8_t 
*default_dp_address)

The advantage is that the function would be simpler to call for the API user 
since we do not need to call an extra free function. The disadvantage is that 
it is more difficult to maintain in case the ASN.1 spec gets extended.

(I just wanted to share some thoughts, as I said, the implementation is 
correct. You do not have to change it if you think it is ok as it is.)



--
To view, visit https://gerrit.osmocom.org/c/onomondo-ipa/+/43031?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: onomondo-ipa
Gerrit-Branch: master
Gerrit-Change-Id: I514b03240289469142e6a41a7290a4afce199318
Gerrit-Change-Number: 43031
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Attention: jolly <[email protected]>
Gerrit-Comment-Date: Wed, 22 Jul 2026 09:53:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Reply via email to