On Tue, Mar 22, 2016 at 10:25 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Christian Couder <christian.couder@xxxxxxxxx> writes: > >> As the value returned by gitdiff_verify_name() is put into the >> same variable that is passed as a parameter to this function, >> it is simpler to pass the address of the variable and have >> gitdiff_verify_name() change the variable itself. >> >> Signed-off-by: Christian Couder <chriscool@xxxxxxxxxxxxx> >> --- > > > This change makes the function less useful by restricting the > callers--only the ones that wants in-place update can call it after > this change, while the old function signature allowed a caller to > pass one variable as orig, receive the result in another variable > (and probably compare them). > > It does not matter very much for this file scope static helper > either way, and I would probably say the same thing if the patch > were in reverse (i.e. if the patch were loosening the restriction), > but I cannot offhand see why this is an improvement. Puzzled... In my opinion it is an improvement because: - the feature of allowing a caller to pass one variable as orig and receive the result in another is useless here and just makes it more difficult to understand what the code is doing because you first have to realize that in both calls the argument that is passed is also assigned to, and - using up the return value to return a result prevents from using it to signal an error instead of die()ing, so it makes libifying the function more difficult. But if you prefer I can just resend a series with only patchs 1/3 and 3/3. And we can discuss later in the context of libifying "git apply" if this patch makes sense. -- 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