Thanks Omar! 

Should we try and catch and return those errors?

-Kyle

> On Nov 18, 2023, at 2:16 AM, Omar Polo <op@omarpolo.com> wrote:
> 
> 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 *