Re: [PATCH 2/2] Fix callsites of real_pathdup() that wanted it to die on error

["René Scharfe" <l.s.r@web.de>]

Am 09.03.2017 um 12:24 schrieb Johannes Schindelin:
> While I would have agreed earlier that René's patch looks less intrusive,
> I have to point out that there would not have been any possible regression
> if the original patch had introduced the die_on_error parameter. It would
> have made the contract *obvious*.
>
> The nicer API made the contract unobvious, and that was the reason that
> the bug could hide.

You mean nicer in the sense of more forgiving, right?

The meaning of calls of a function with a die_on_error parameter are 
only obvious if at least the name of that parameter is known.  A 
declaration without it would not suffice:

	char *real_pathdup(const char *, int);

There are two possibilities that can't be distinguished by looking at 
callers alone:

	char *real_pathdup(const char *path, int die_on_error);
	char *real_pathdup(const char *path, int gentle);

An enum can help, so that calls look something like this:

	copy_or_death = real_pathdup(path, DIE_ON_ERROR);
	copy = real_pathdup(path, IGNORE_ERRORS); if (copy) ...

For binary flags we can easily encode that information into the function 
names and thus simplify the API:

	copy_or_death = real_pathdup_or_die(path);
	copy = real_pathdup_gently(path); if (copy) ...

And we can drop the suffix of the variant used overwhelmingly often, but 
as you pointed out that relies on readers knowing that convention.

I'm sure Brandon will balance these concerns nicely in follow-up patches. ;)

René