On Wed, Aug 21, 2013 at 05:49:24PM -0400, Aaron Schrab wrote:
> Clear newly allocated error buffer to avoid attempt to dereference an
> invalid pointer when reporting an error while sourcing a file. Without
> this change I was seeing segfaults when attempting to source a file
> containing a send2-hook with an invalid regexp in the pattern.
> ---
> commands.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/commands.c b/commands.c
> index 13b12dd..fea9408 100644
> --- a/commands.c
> +++ b/commands.c
> @@ -618,6 +618,7 @@ void mutt_enter_command (void)
> buffer[0] = 0;
> if (mutt_get_field (":", buffer, sizeof (buffer), M_COMMAND) != 0 ||
> !buffer[0])
> return;
> + memset (&err, 0, sizeof (err));
> err.dsize = STRING;
> err.data = safe_malloc(err.dsize);
> memset (&token, 0, sizeof (token));
Given this code and the fix, it seems likely that the problem you ran
into is that err.dptr is used uninitialized. Since err is half
initialized in the code that follows, I personally think it would be
slightly preferable to add:
err.dptr = NULL;
/* not sure if this is used, but that's not a reason to skip initializing it
*/
err.destroy = 0;
It's slighly more code, but it's clearer and avoids assigning
err.dsize and err.data twice.
But that said, your fix seems fine.
THAT said, I'm betting there are other places in the code where there
are BUFFER structs which aren't initialized properly. Might be worth
creating a function/macro to initialize these things properly... e.g.:
initialize_buffer(BUFFER *b, size_t size, int destroy)
{
b->dsize = size;
b->data = SOME_MALLOC(size);
b->dptr = NULL;
b->destroy = destroy;
}
--
Derek D. Martin http://www.pizzashack.org/ GPG Key ID: 0xDFBEAD02
-=-=-=-=-
This message is posted from an invalid address. Replying to it will result in
undeliverable mail due to spam prevention. Sorry for the inconvenience.
pgp09QTZm5c_4.pgp
Description: PGP signature
