Re: [PATCH] error out if path is invalid

[Dmitry Potapov <dpotapov@gmail.com>]

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

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?


Dmitry