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