Christian Couder <christian.cou...@gmail.com> writes:

> Signed-off-by: Christian Couder <chrisc...@tuxfamily.org>
> ---
>  builtin/apply.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)

To be honest, I have to say that I do not like this change from
readability's point of view.  Once the global p_value becomes a
field in a "apply state" structure, functions will get a pointer to
the struct as a parameter, reference to it would be spelled as
"astate->p_value", while a local variable or a parameter would be
"p_value" and there is no shadowing issue.

Also, you wouldn't be able to catch a misconversion in this patch
only from the patch text, if the conversion is done this way, would
you?  The patch may change the parameter p_value to p_v, and change
two references to p_value also to p_v, but it may leave another
reference to p_value that originally referred to the parameter
as-is, making it refer to the global, which would be a new bug
introduced by this conversion patch, but that remaining reference
would not show up in the patch.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to