Web lists-archives.com

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




Brandon Williams <bmwill@xxxxxxxxxx> writes:

>> > diff --git a/abspath.c b/abspath.c
>> > index 2f0c26e0e2c..b02e068aa34 100644
>> > --- a/abspath.c
>> > +++ b/abspath.c
>> > @@ -214,12 +214,12 @@ const char *real_path_if_valid(const char *path)
>> >  	return strbuf_realpath(&realpath, path, 0);
>> >  }
>> >  
>> > -char *real_pathdup(const char *path)
>> > +char *real_pathdup(const char *path, int die_on_error)
>> 
>> Adding a gentle variant (with the current implementation) and making
>> real_pathdup() die on error would be nicer, as it doesn't require
>> callers to pass magic flag values.  Most cases use the dying variant,
>> so such a patch would have to touch less places:
>
> I agree with Junio and Rene that a gentle version would make the api
> slightly nicer (and more consistant with some of the other api's we have
> in git).
>
> This is exactly what I should have done back when I originally made the
> change.  Sorry for missing this!

While I agree that the shape of the code Rene gave us here is what
we would have liked to have in the original, it is a bit too late
for that.

As I already mentioned, as a regression fix patch, I find what Dscho
posted more sensible, because it makes it obvious that all existing
callsites were looked at while constructing the patch and more
importantly, it forces somebody to look at all the new callers of
the function that were added by the topics in flight, by changing
the func-signature and forcing compilation failure.

It may be somewhat unfortunate that that somebody needs to be me,
but that is what maintainers are for, so ... ;-)

Once the dust settles, let's do the clean-up along the lines of
Rene's patch.

Thanks.