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
--
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
