Re: [PATCH 1/4] add generic, type aware object chain walker
- From
Shawn O. Pearce <spearce@spearce.org>
- Date
- Feb 25, 2008, 08:04 UTC
- Message-ID
- <20080225080445.GM8410@spearce.org>
- In-Reply-To
- <20080225075242.GD15761@auto.tuwien.ac.at>
Martin Koegler <mkoegler@auto.tuwien.ac.at> wrote:
> > What about > #define OBJ_BAD -2
All of the other OBJ_* constants are declared in the enum object_type, not as #defines.
See below for why this should not be <0.
Show 18 quoted lines
> > After all, if the walker callback was told with
> > OBJ_ANY that any type of object is Ok, it should still say
> > "oops" if the given object said it actually is of type OBJ_BAD.
> > E.g. in your [2/4] patch:
> >
> > +static int mark_object(struct object *obj, int type, void *data)
> > +{
> > + ...
> > + if (type != OBJ_ANY && obj->type != type) {
> > + objerror(parent, "wrong object type in link");
> > + }
> >
> > if you use the above #define, a tagged object that has a bad
> > type will pass this check unnoticed, won't it?
>
> No, it wouldn't, as object->type is never initialized to OBJ_BAD:
> $ grep "OBJ_BAD" *.c *.h
> cache.h: OBJ_BAD = -1,Today that may be true. OBJ_BAD was created for cases where the internal object header parsing code in say sha1_file.c finds a type code it doesn't recognize and wants to return it back to the caller. This grew out of some of the early pack v4 work.
It actually happens today when a function that returns an object type enum returns failure. sha1_object_info() is one such function. It returns "int" but that int is also really an enum object_type. When that function fails it returns -1, which has it happens is OBJ_BAD.
Subtle, yes. But OBJ_BAD has meaning and is in use today. Please do not use it for "we don't care what it is".
-- Shawn.