Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>> @@ -1254,9 +1273,26 @@ static void emit(const char *cp, const char *ep)
>> +static void reset_formatting_state(struct ref_formatting_state *state)
>> +{
>> + int quote_style = state->quote_style;
>> + memset(state, 0, sizeof(*state));
>> + state->quote_style = quote_style;
>
> I wonder if this sledge-hammer approach of saving one or two values
> before clearing the entire 'ref_formatting_state' and then restoring
> the saved values will scale well. Would it be better for this to just
> individually reset the fields which need resetting and not touch those
> that don't?
>
> Also, the fact that quote_style has to be handled specially may be an
> indication that it doesn't belong in this structure grouped with the
> other modifiers or that you need better classification within the
> structure.
Actually, I think it is wrong to have this function in the first
place. It is a sign that the caller is doing too little before
calling this function.
If the act of printing an atom uses the formatting state that says
"next one needs X", then it is responsible to clear that "next one
needs X" part of the state, as it is the one who consumed that
state. E.g. if it used to say "next one needs to be padded to the
right" before entering print_value(), then the function did that
"padded output", then the "next one needs to be padded to the
right" should be cleared inside print_value().
--
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