avoid segfaults on parse_object failure

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>

diff --git a/bundle.c b/bundle.c
index 8d12816b9d0bc682ed9c019a7a5d5cec4b859171..26ceebdb4bdd22949ac35acb5e07b3bce6f22339 100644 (file)
--- a/bundle.c
+++ b/bundle.c
@@ -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)) {
                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)) {
                                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;
                }
        }
                        object->flags |= SHOWN;
                }
        }


@@ -361,7 +361,7 @@ int create_bundle(struct bundle_header *header, const char *path,
                                 * end up triggering "empty bundle"
                                 * error.
                                 */
                                 * 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);
                        }
                                obj->flags |= SHOWN;
                                add_pending_object(&revs, obj, e->name);
                        }

diff --git a/object.c b/object.c
index 4af3451bf89471a0ab7a148aad4924a8ac8ae053..20703f52ed24aa227d451bec332ac73cc3d49d3a 100644 (file)
--- a/object.c
+++ b/object.c
@@ -185,6 +185,16 @@ struct object *parse_object_buffer(const unsigned char *sha1, enum object_type t
        return obj;
 }
 
        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;
 struct object *parse_object(const unsigned char *sha1)
 {
        unsigned long size;

diff --git a/object.h b/object.h
index 6a97b6ba1a43e1d38eb07515ad298e0067628127..97d384b80a5dbf0cac88b59cbb8a2d333d2b9582 100644 (file)
--- a/object.h
+++ b/object.h
@@ -54,9 +54,20 @@ struct object *lookup_object(const unsigned char *sha1);
 
 extern void *create_object(const unsigned char *sha1, int type, void *obj);
 
 
 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);
 
 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.
 /* 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.

diff --git a/pack-refs.c b/pack-refs.c
index f09a05422854c919cd31c542b82b057624275959..6a689f35cba707c7afa979d699208ef55eae5f78 100644 (file)
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -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) {
 
        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)
                if (o->type == OBJ_TAG) {
                        o = deref_tag(o, path, 0);
                        if (o)