On 2023/11/18 00:48:43 -0600, Kyle Ackerman <kackerman0102@gmail.com> wrote:
> Here is a diff for lib/privsep.c that fixes a memory leak spotted by
> Omar.  Any suggestions?

Seems correct to me, thank you!

Two nitpics:

- we use tabs (8 columns) instead of spaces for indentations

> [...]
>                 datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
>                 if (imsg.hdr.type == GOT_IMSG_COMMIT_PAINTING_DONE)
>                         break;

 - here it leaks too.  I've added an imsg_free() call here too before
   committing.


Reviewing your diff I've also spotted another small issue:

   3540                 for (i = 0; i < icommits.ncommits; i++) {
   ....
   3545                         if (icommits.present_in_pack) {
   ....
   3548                                 err = cb(cb_arg, &id, icommit.color);
   3549                                 if (err)
   3550                                         break;

err is set here

   3551                         } else {
   3552                                 struct got_object_qid *qid;
   3553                                 err = got_object_qid_alloc_partial(&qid);
   3554                                 if (err)
   3555                                         break;

and here too

   ....
   3562
   3563                 imsg_free(&imsg);
   3564         }

but then is ignored since we continue looping.

diff /home/op/w/got
commit - 6d0030ba1d9985fc8b8a9a2dc0f0c412c2c678da
path + /home/op/w/got
blob - 2109bc13bcb9c7dbfb07021a640c5fa125e74954
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -3518,7 +3518,7 @@ got_privsep_recv_painted_commits(struct got_object_id_
 		datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
 		if (imsg.hdr.type == GOT_IMSG_COMMIT_PAINTING_DONE) {
 			imsg_free(&imsg);
-			break;
+			return NULL;
 		}
 		if (imsg.hdr.type != GOT_IMSG_PAINTED_COMMITS){
 			imsg_free(&imsg);
@@ -3561,9 +3561,9 @@ got_privsep_recv_painted_commits(struct got_object_id_
 		}
 
 		imsg_free(&imsg);
+		if (err)
+			return err;
 	}
-
-	return err;
 }
 
 const struct got_error *