On Fri, Mar 20, 2009 at 10:11:27AM -0700, Keith Cascio wrote:
> (a) numeric data (integers, chars, and floats): define magic value(s)
> that represent pristineness. Initialize all fields to PRISTINE.
> Later, if a field has any value other than PRISTINE, we know there was
> intent-to-change.
Good point. Though we will need to make sure that existing code is never
looking at PRISTINE values, which aren't likely to make much sense (I
suspect most will be INT_MAX or -1, as 0 is a reasonable value for many
of the options). This should be easy for most code, since the flattening
will get rid of PRISTINE. But remember that there are pieces of code
that do something like:
if (some_diff_option_is_set)
set_some_other_related_diff_option;
which will need to be PRISTINE-aware.
> > 3. Introduce some mechanism to help future callers get it right
> > (since otherwise assigning directly is a subtle bug).
>
> Yes, in the future, someone could write naughty code that sets a bit
> flag directly rather than using one of the macros. In C, we probably
> can't make that impossible. But generally speaking we can't protect
> against all forms of gross negligence.
I think you can safely ignore this complaint. I was worried we would
need something like:
DIFF_SET(&opt, stat_name_width, 10);
It is much easier to mistakenly write this as
opt.state_name_width = 10;
than it is to accidentally do a bit-set when there is a DIFF_OPT_SET
macro. That is, I think most people _want_ to use DIFF_OPT_SET because
it is easier to read and less typing.
So I saw this as a problem more for non-bit options, but you have
already addressed that above.
> All in all, turns out v3 requires surprisingly little modification of
> existing code outside of diff.h/diff.c. Actually, it only adds 3
> lines, that's it!!
>
> builtin-diff.c | 2 +
> builtin-log.c | 1 +
> diff.c | 112 ++++++++++++++++++++++++-
> diff.h | 17 +++-
>
> Shall I post v3?
Yes, please. It is much better to be talking about actual code than
hypotheticals.
-Peff
--
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
