On 22-05-2025 11:55 pm, Eric Sunshine wrote:

On Thu, May 22, 2025 at 2:21 PM Aditya Garg [off-list ref] wrote:

quoted

On 22-05-2025 11:30 pm, Eric Sunshine wrote:

quoted

Squinting at the code a bit more, am I correct in thinking that
6d1f198f34 missed a case and that the function is still leaking
`cfg->host` in the "imap.host" conditional? I haven't traced the code
or all the callers, but I wonder if server_fill_credential() in the
same file may also be leaky. In any event, the `cfg->host` and the
possible server_fill_credential() leaks are outside the scope of this
bug-fix patch.

Not sure about server_fill_credential(), but I think this is also
a potential memory leak

Agreed.

quoted

static int auth_cram_md5(struct imap_store *ctx, const char *prompt)
{
        int ret;
        char *response;

        response = cram(prompt, ctx->cfg->user, ctx->cfg->pass);

        ret = socket_write(&ctx->imap->buf.sock, response, strlen(response));
        if (ret != strlen(response))
+               free(response); // fix for the leak
                return error("IMAP error: sending response failed");

        free(response);

        return 0;
}

It's subjective, but I would probably fix this a little bit
differently and (to my mind) more simply:

    response = cram(prompt, ctx->cfg->user, ctx->cfg->pass);

    ret = socket_write(&ctx->imap->buf.sock, response, strlen(response));
    free(response);
    if (ret != strlen(response))
        return error("IMAP error: sending response failed");
    return 0;

Looks equally good. Such minor fixes can be included in this series.