Re: [PATCH] error out if path is invalid

[Johannes Sixt <j.sixt@viscovery.net>]

Dmitry Potapov schrieb:
> On Mon, Oct 06, 2008 at 09:02:22AM +0200, Johannes Sixt wrote:
>> Dmitry Potapov schrieb:
>>>  	if (!verify_path(path))
>>> -		return -1;
>>> +		return error("Invalid path '%s'", path);
>> Look at this change. Didn't the code error out before, too?
> 
> It is certainly did not here. As to its caller, it depends. In fact,
> there are two chunks like that in my patch, so I am not sure to which
> one you refer here. If we speak about add_cacheinfo() then though the
> function did not error out, its caller died with one of the following
> messages:
>   git update-index: unable to update some-file-name
> or
>   git update-index: --cacheinfo cannot add some-file-name

Look at the original patch. You did not change the behavior except to
write more error messages. Maybe I misunderstand the words "to error out".
I understand them as "to detect an error and return early", but not "write
an error message".

> However, if we speak about add_index_entry_with_check then the caller
> will not produce any error. The git would exit successfully (it still
> does) and there was no error message as if everything was fine.
> 
> Perhaps, the exit code should be corrected too, but if the git just dies
> when add_index_entry() fails it may cause that having one invalid path
> will prevent to check out other files, which does not seem to be the
> right thing to do.
> 
> As to correction to correction to make_cache_entry then after my
> previous patch, it started to error out:
> 
>   make_cache_entry failed for path 'some-file-name'
> 
> before that it silently segfaulted.
> 
>> Same in the
>> other cases. Hence, your patch subject does not describe the patch.
> 
> Should I include the above explanation in the commit message or do you
> have any objection to having the above error message in cases where the
> caller already produce some message when it dies?

I don't object the change, only its (missing or IMHO incorrect)
justification. I don't think that the above text would be the correct
description because as far as I can see the only change you made was to
add error messages.

-- Hannes