Skip to content

Commit

Permalink
avoid segfaults on parse_object failure
Browse files Browse the repository at this point in the history
Many call-sites of parse_object assume that they will get a
non-NULL return value; this is not the case if we encounter
an error while parsing the object.

This patch adds a wrapper function around parse_object that
handles dying automatically, and uses it anywhere we
immediately try to access the return value as a non-NULL
pointer (i.e., anywhere that we would currently segfault).

This wrapper may also be useful in other places. The most
obvious one is code like:

  o = parse_object(sha1);
  if (!o)
	  die(...);

However, these should not be mechanically converted to
parse_object_or_die, as the die message is sometimes
customized. Later patches can address these sites on a
case-by-case basis.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
peff authored and gitster committed Mar 17, 2013
1 parent 7e20105 commit 75a9549
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 5 deletions.
6 changes: 3 additions & 3 deletions bundle.c
Expand Up @@ -279,12 +279,12 @@ int create_bundle(struct bundle_header *header, const char *path,
if (buf.len > 0 && buf.buf[0] == '-') {
write_or_die(bundle_fd, buf.buf, buf.len);
if (!get_sha1_hex(buf.buf + 1, sha1)) {
struct object *object = parse_object(sha1);
struct object *object = parse_object_or_die(sha1, buf.buf);
object->flags |= UNINTERESTING;
add_pending_object(&revs, object, xstrdup(buf.buf));
}
} else if (!get_sha1_hex(buf.buf, sha1)) {
struct object *object = parse_object(sha1);
struct object *object = parse_object_or_die(sha1, buf.buf);
object->flags |= SHOWN;
}
}
Expand Down Expand Up @@ -361,7 +361,7 @@ int create_bundle(struct bundle_header *header, const char *path,
* end up triggering "empty bundle"
* error.
*/
obj = parse_object(sha1);
obj = parse_object_or_die(sha1, e->name);
obj->flags |= SHOWN;
add_pending_object(&revs, obj, e->name);
}
Expand Down
10 changes: 10 additions & 0 deletions object.c
Expand Up @@ -185,6 +185,16 @@ struct object *parse_object_buffer(const unsigned char *sha1, enum object_type t
return obj;
}

struct object *parse_object_or_die(const unsigned char *sha1,
const char *name)
{
struct object *o = parse_object(sha1);
if (o)
return o;

die(_("unable to parse object: %s"), name ? name : sha1_to_hex(sha1));
}

struct object *parse_object(const unsigned char *sha1)
{
unsigned long size;
Expand Down
13 changes: 12 additions & 1 deletion object.h
Expand Up @@ -54,9 +54,20 @@ struct object *lookup_object(const unsigned char *sha1);

extern void *create_object(const unsigned char *sha1, int type, void *obj);

/** Returns the object, having parsed it to find out what it is. **/
/*
* Returns the object, having parsed it to find out what it is.
*
* Returns NULL if the object is missing or corrupt.
*/
struct object *parse_object(const unsigned char *sha1);

/*
* Like parse_object, but will die() instead of returning NULL. If the
* "name" parameter is not NULL, it is included in the error message
* (otherwise, the sha1 hex is given).
*/
struct object *parse_object_or_die(const unsigned char *sha1, const char *name);

/* Given the result of read_sha1_file(), returns the object after
* parsing it. eaten_p indicates if the object has a borrowed copy
* of buffer and the caller should not free() it.
Expand Down
2 changes: 1 addition & 1 deletion pack-refs.c
Expand Up @@ -40,7 +40,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,

fprintf(cb->refs_file, "%s %s\n", sha1_to_hex(sha1), path);
if (is_tag_ref) {
struct object *o = parse_object(sha1);
struct object *o = parse_object_or_die(sha1, path);
if (o->type == OBJ_TAG) {
o = deref_tag(o, path, 0);
if (o)
Expand Down

0 comments on commit 75a9549

Please sign in to comment.