Re: [PATCH 26/67] replace trivial malloc + sprintf /strcpy calls to xstrfmt

On Wed, Sep 16, 2015 at 12:24:38AM -0400, Eric Sunshine wrote:

On Tue, Sep 15, 2015 at 11:45 AM, Jeff King [off-list ref] wrote:

quoted

replace trivial malloc + sprintf /strcpy calls to xstrfmt

s/to/with/

Also, do you want either to add a space after '/' or drop the one before it?

Thanks, fixed both.

quoted

--- a/imap-send.c
+++ b/imap-send.c

@@ -889,9 +889,8 @@ static char *cram(const char *challenge_64, const char *user, const char *pass)
        }

        /* response: "<user> <digest in hex>" */
-       resp_len = strlen(user) + 1 + strlen(hex) + 1;
-       response = xmalloc(resp_len);
-       sprintf(response, "%s %s", user, hex);
+       response = xstrfmt("%s %s", user, hex);
+       resp_len = strlen(response);

        response_64 = xmalloc(ENCODED_SIZE(resp_len) + 1);

The original resp_len calculation included the NUL but the revised
does not. If I'm reading this correctly, the revised calculation is
correct, and the original was over-allocating response_64, right?

Hrm, I didn't notice that. We actually feed "resp_len" directly to
EVP_EncodeBlock, meaning it it _would_ include the trailing NUL in the
encoded output. So my modification is not wrong memory-wise (we allocate
enough to hold the result), but does produce different output.

I'm not at all convinced that the original is correct, but if we are
going to fix it, it should definitely not be part of this patch. I'll
switch it to:

  resp_len = strlen(response) + 1;

which matches the original.

Thanks for being such a careful reviewer. :)

-Peff