On Thu, Aug 5, 2010 at 13:17, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote: > ll_merge() takes its options in a flag word, which has a few > advantages: > > - options flags can be cheaply passed around in registers, while > an option struct passed by pointer cannot; > > - callers can easily pass 0 without trouble for no options, > while an option struct passed by value would not allow that. > > The downside is that code to populate and access the flag word can be > somewhat opaque. Mitigate that with a few macros. > > Cc: Avery Pennarun <apenwarr@xxxxxxxxx> > Cc: Bert Wesarg <bert.wesarg@xxxxxxxxxxxxxx> > Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> > --- > Documentation/technical/api-merge.txt | 11 +++++++---- > ll-merge.c | 9 +++++---- > ll-merge.h | 14 ++++++++++++++ > merge-recursive.c | 3 ++- > 4 files changed, 28 insertions(+), 9 deletions(-) > > diff --git a/Documentation/technical/api-merge.txt b/Documentation/technical/api-merge.txt > index 01a89d6..a7e050b 100644 > --- a/Documentation/technical/api-merge.txt > +++ b/Documentation/technical/api-merge.txt > @@ -49,12 +49,15 @@ supports this. > > The `flag` parameter is a bitfield: > > - - The least significant bit indicates whether this is an internal > - merge to consolidate ancestors for a recursive merge. > + - The `LL_OPT_VIRTUAL_ANCESTOR` bit indicates whether this is an > + internal merge to consolidate ancestors for a recursive merge. > > - - The next two bits allow local conflicts to be automatically > + - The `LL_OPT_FAVOR_MASK` bits allow local conflicts to be automatically > resolved in favor of one side or the other (as in 'git merge-file' > - `--ours`/`--theirs`/`--union` for 01, 10, and 11, respectively). > + `--ours`/`--theirs`/`--union`). > + They can be populated by `create_ll_flag`, whose argument can be > + `XDL_MERGE_FAVOR_OURS`, `XDL_MERGE_FAVOR_THEIRS`, or > + `XDL_MERGE_FAVOR_UNION`. > > Everything else > --------------- > diff --git a/ll-merge.c b/ll-merge.c > index 5068fe0..290f764 100644 > --- a/ll-merge.c > +++ b/ll-merge.c > @@ -46,7 +46,7 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused, > * or common ancestor for an internal merge. Still return > * "conflicted merge" status. > */ > - mmfile_t *stolen = (flag & 01) ? orig : src1; > + mmfile_t *stolen = (flag & LL_OPT_VIRTUAL_ANCESTOR) ? orig : src1; > > result->ptr = stolen->ptr; > result->size = stolen->size; > @@ -79,7 +79,7 @@ static int ll_xdl_merge(const struct ll_merge_driver *drv_unused, > > memset(&xmp, 0, sizeof(xmp)); > xmp.level = XDL_MERGE_ZEALOUS; > - xmp.favor= (flag >> 1) & 03; > + xmp.favor = ll_opt_favor(flag); > if (git_xmerge_style >= 0) > xmp.style = git_xmerge_style; > if (marker_size > 0) > @@ -99,7 +99,8 @@ static int ll_union_merge(const struct ll_merge_driver *drv_unused, > int flag, int marker_size) > { > /* Use union favor */ > - flag = (flag & 1) | (XDL_MERGE_FAVOR_UNION << 1); > + flag = (flag & LL_OPT_VIRTUAL_ANCESTOR) | > + create_ll_flag(XDL_MERGE_FAVOR_UNION); > return ll_xdl_merge(drv_unused, result, path_unused, > orig, NULL, src1, NULL, src2, NULL, > flag, marker_size); > @@ -342,7 +343,7 @@ int ll_merge(mmbuffer_t *result_buf, > const char *ll_driver_name = NULL; > int marker_size = DEFAULT_CONFLICT_MARKER_SIZE; > const struct ll_merge_driver *driver; > - int virtual_ancestor = flag & 01; > + int virtual_ancestor = flag & LL_OPT_VIRTUAL_ANCESTOR; > > if (merge_renormalize) { > normalize_file(ancestor, path); > diff --git a/ll-merge.h b/ll-merge.h > index 57754cc..5990271 100644 > --- a/ll-merge.h > +++ b/ll-merge.h > @@ -5,6 +5,20 @@ > #ifndef LL_MERGE_H > #define LL_MERGE_H > > +#define LL_OPT_VIRTUAL_ANCESTOR (1 << 0) > +#define LL_OPT_FAVOR_MASK ((1 << 1) | (1 << 2)) > +#define LL_OPT_FAVOR_SHIFT 1 > + > +static inline int ll_opt_favor(int flag) > +{ > + return (flag & LL_OPT_FAVOR_MASK) >> LL_OPT_FAVOR_SHIFT; > +} > + > +static inline int create_ll_flag(int favor) > +{ > + return ((favor << LL_OPT_FAVOR_SHIFT) & LL_OPT_FAVOR_MASK); > +} > + These two function names do not suggests that these are symmetric. How about get_ll_flavor() and create_ll_flavor()? Or flavor_to_ll_flag() and ll_flag_to_flavor(). Regards, Bert > int ll_merge(mmbuffer_t *result_buf, > const char *path, > mmfile_t *ancestor, const char *ancestor_label, > diff --git a/merge-recursive.c b/merge-recursive.c > index 8a49844..c0c9f0c 100644 > --- a/merge-recursive.c > +++ b/merge-recursive.c > @@ -647,7 +647,8 @@ static int merge_3way(struct merge_options *o, > > merge_status = ll_merge(result_buf, a->path, &orig, base_name, > &src1, name1, &src2, name2, > - (!!o->call_depth) | (favor << 1)); > + ((o->call_depth ? LL_OPT_VIRTUAL_ANCESTOR : 0) | > + create_ll_flag(favor))); > > free(name1); > free(name2); > -- > 1.7.2.1.544.ga752d.dirty > > -- 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