On Sun, Jun 29, 2008 at 10:40:53PM -0700, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> > Actually reset_hard does not return if an error occures:
>
> I know that; didn't I already say "Luckily no"? The point was it was not
> apparent from the above 6 lines alone.
Ah, OK.
>
> >> > + for (i = 0; i < use_strategies.nr; i++) {
> >> > + if ((unsigned int)use_strategies.items[i].util &
> >> > + NO_FAST_FORWARD)
> >> > + allow_fast_forward = 0;
> >> > + if ((unsigned int)use_strategies.items[i].util & NO_TRIVIAL)
> >> > + allow_trivial = 0;
> >>
> >> Can we abstract out these ugly casts? Any code that use path_list to
> >> store anything but list of paths (i.e. some value keyed with string) tends
> >> to have this readability issue.
> >
> > If you don't cast, you can't use the & operator. If I change the
> > path_list_item's util to be an unsigned number then I break fast-export.
> > I think if we _really_ want to get rid of those casts, we could have
> > something like:
>
> No, no, no. That is not what I meant.
>
> The places that use use_strategies in your code knows too much about the
> internal implementation detail of path_list, while path_list pretends to
> be a general purpose "table keyed with string" facility. The fact is that
> the table is not a very useful general purpose abstraction unless you are
> pointing at some structures that exist regardless of your use of path_list
> (e.g. you have some "struct object" and you hold pointers in a path_list).
> It does not work very well as an abstraction for use case like yours.
>
> With something like:
>
> static inline unsigned nth_strategy_flags(struct path_list *s, int nth)
> {
> return (unsigned) s->items[nth].util;
> }
>
> the checks would be more like:
>
> if (nth_strategy_flags(&use_strategies, i) & NO_FAST_FORWARD)
> ...
>
> or even:
>
> static inline check_nth_strategy_flags(struct path_list_item *i, unsigned flags)
> {
> return !((unsigned) i->util & flags);
> }
>
> if (check_nth_strategy_flags(&use_strategies,items[i], NO_FAST_FORWARD)
> ...
>
> either of which would be much easier on the eye.
Probably this is subjective, but I think the previous form is easier to
read, so I choose that one.
Attachment:
pgpyW74aWyTiT.pgp
Description: PGP signature
