@henningw commented on this pull request.

Thanks for the feedback. Regarding the commit message, I misread it, sorry. I looked to the code, and add a few remarks. Please have a look, thank you.


In src/modules/ims_registrar_scscf/regpv.c:

> @@ -440,7 +451,15 @@ int pv_fetch_contacts(
 		return -1;
 	}
 
-	ptr = 0; //r->contacts;TODO
+	rpp->impi.s = (char *)pkg_malloc(r->private_identity.len * sizeof(char));
+	if(rpp->impi.s == NULL) {
+		LM_ERR("no mem for impi\n");
+		return -1;

Do we need to free the earlier allocated memory here? Maybe also just jump to error?


In src/modules/ims_registrar_scscf/regpv.c:

> @@ -455,8 +474,8 @@ int pv_fetch_contacts(
 			goto error;
 		}
 		memcpy(c0, ptr, ilen);
-		//c0->domain = {0,0};//NULL;TODO
-		//c0->aor = {0,0};//NULL;
+		c0->domain = (str){NULL, 0};

I don't think you need the casts here, it would be probably more consistent to use the {0, 0} as elsewhere in this function.


In src/modules/ims_registrar_scscf/regpv.c:

> @@ -440,7 +451,15 @@ int pv_fetch_contacts(
 		return -1;
 	}
 
-	ptr = 0; //r->contacts;TODO
+	rpp->impi.s = (char *)pkg_malloc(r->private_identity.len * sizeof(char));
+	if(rpp->impi.s == NULL) {
+		LM_ERR("no mem for impi\n");
+		return -1;

I think a similar issue is probably in line 439?


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: <kamailio/kamailio/pull/4166/review/2743923640@github.com>