Re: [PATCH 1/4] add generic, type aware object chain walker

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



mkoegler@xxxxxxxxxxxxxxxxx (Martin Koegler) writes:

> On Sun, Feb 24, 2008 at 10:04:04PM -0500, Shawn O. Pearce wrote:
>> Martin Koegler <mkoegler@xxxxxxxxxxxxxxxxx> wrote:
>> Hmm.  Don't you need to get commit->parenst *after* it is parsed,
>> and not before?
>
> Thanks, fixed.
>
>> > @@ -0,0 +1,10 @@
>> > +#ifndef GIT_FSCK_H
>> > +#define GIT_FSCK_H
>> > +
>> > +#define OBJ_ANY OBJ_BAD
>> 
>> Its unclear why this macro is necessary.
>
> There are cases (eg. a tag->tagged), where the object type does not
> matter. In this case, the callback gets this value for the expected
> type.
>
> The value of OBJ_BAD (-1) is not used anywhere in git, so its
> free. OBJ_ANY is just a better name.
>
> If somebody has a better idea, please tell me.

As I mentioned in my previous review comment, the token is used
to tell the walker callback function that this type of object
was expected where it was found, in order to allow the callback
to check the type of the object.  I think OBJ_ANY is a very good
name for the token you would use to tell the callback that any
type of object can appear (because that is a taggee).

So I do not have objection to OBJ_ANY (but again, this kind of
thing needs to be explained in your commit log message), but
giving the token the same value as OBJ_BAD may not be such a
cool idea.  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?
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux