From: Stefan Beller <stefanbeller@googlemail.com>
To: Philip Oakley <philipoakley@iee.org>
Cc: Jeff King <peff@peff.net>, Johannes Sixt <j.sixt@viscovery.net>,
Ramsay Jones <ramsay@ramsay1.demon.co.uk>,
Junio C Hamano <gitster@pobox.com>,
GIT Mailing-list <git@vger.kernel.org>
Subject: Re: [PATCH] Fix some sparse warnings
Date: Thu, 18 Jul 2013 00:08:24 +0200 [thread overview]
Message-ID: <51E715D8.9040307@googlemail.com> (raw)
In-Reply-To: <6BDA2E3E7318418BBB2C19B475B2B118@PhilipOakley>
[-- Attachment #1: Type: text/plain, Size: 1903 bytes --]
On 07/16/2013 10:53 PM, Philip Oakley wrote:
>
> Does anyone run the "new static checker called 'Stack' that precisely
> identifies unstable code"? [though the paper's conclusion says 'All
> Stack source code will be publicly available.' which suggests it's not
> yet available]
>
So I started using the clang code analyzer on git. One of the
first warnings actually is this:
object.c:241:7: warning: Branch condition evaluates to a garbage value
if (!eaten)
So that part of object.c lookx like this:
struct object *parse_object(const unsigned char *sha1)
{
int eaten;
...
obj = parse_object_buffer(sha1, type, size, buffer, &eaten);
if (!eaten)
free(buffer);
}
And the parse_object_buffer looks like this with respect to the eaten
variable:
struct object *parse_object_buffer(...)
{
int eaten = 0;
if (something)
return NULL;
...
if (something_different)
eaten=1;
*eaten_p = eaten;
}
So what might happen is, that parse_object_buffer exits early, without
executing
*eaten_p = eaten;
Then in the parse_object function eaten was never initialized nor set
inside the call to parse_object_buffer. Then it is obvious that the
free(buffer) is executed depending on garbage left on the stack.
Definitely something what we want to change.
The obvious way to repair this would be to just initialize the eaten variable
inside parse_object.
struct object *parse_object(const unsigned char *sha1)
{
int eaten=0;
...
However I'd like to propose another solution:
In parse_object_buffer we do not have a local eaten variable, but
directly write to *eaten_p. That would be the following patch.
Was there a particular idea or goal behind first having a local eaten
variable, which later near the correct return of the function was used to set the
eaten_p?
Thanks,
Stefan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
next prev parent reply other threads:[~2013-07-17 22:08 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-15 17:31 [PATCH] Fix some sparse warnings Ramsay Jones
2013-07-16 5:57 ` Johannes Sixt
2013-07-16 6:21 ` Jeff King
2013-07-16 20:53 ` Philip Oakley
2013-07-16 21:18 ` Stefan Beller
2013-07-16 22:18 ` Philip Oakley
2013-07-17 5:47 ` Johannes Sixt
2013-07-17 22:08 ` Stefan Beller [this message]
2013-07-17 22:09 ` [PATCH] parse_object_buffer: Correct freeing the buffer Stefan Beller
2013-07-17 22:16 ` [PATCH] Fix some sparse warnings Stefan Beller
2013-07-17 23:22 ` Junio C Hamano
2013-07-18 17:58 ` Ramsay Jones
-- strict thread matches above, loose matches on Subject: below --
2013-07-18 20:25 Ramsay Jones
2013-07-18 20:36 ` Jeff King
2013-07-20 19:26 ` Ramsay Jones
2013-07-21 17:39 ` Jonathan Nieder
2013-07-21 20:58 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51E715D8.9040307@googlemail.com \
--to=stefanbeller@googlemail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j.sixt@viscovery.net \
--cc=peff@peff.net \
--cc=philipoakley@iee.org \
--cc=ramsay@ramsay1.demon.co.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).