Re: [PATCH] Make the exit code of add_file_to_index actually useful

Junio C Hamano, Mon, May 12, 2008 20:42:58 +0200:

quoted

I corrected the series to use die() again and rebased it off current
master (65ea3b8c). So it is more compatible with libification (does
not hinder it more than previos code) and keep the exit code.

But you did not answer my question in the part you did not quote, did you?

I believe I did:

Date:	Sun, 2 Mar 2008 22:42:41 +0100
Subject: Re: [PATCH] Make the exit code of add_file_to_index actually useful
Message-ID: [ref]

Junio C Hamano, Sun, Mar 02, 2008 17:59:13 +0100:

Why is this even needed to begin with?  I am aware of Dirk's original
issue discussed elsewhere, but we try fairly hard to be A-O-N when we can
afford to, and this option deliberately breaks it.  What is the real
reason why such an unreadable (either for privilege or for I/O error)
file should not live in .gitignore?

Another program keeps the file open. There is an exclusive mode for
opening files, which locks the files for everyone. I believe it is
even default mode, unless selected otherwise.

Now when somebody either forgets to check the return value from this
function, or deliberately ignores it, the resulting index will not match
what the code is told to update it with.

I think I got them all in the current code:

    $git grep -E 'add_(file_)?to_(index|cache)'
    Documentation/technical/api-in-core-index.txt:* add_file_to_index()
    builtin-add.c:                  if (add_file_to_cache(path, data->flags
    builtin-add.c:          if (add_file_to_cache(dir.entries[i]->name, ver
    builtin-commit.c:                       if (add_to_cache(p->path, &st,
    builtin-mv.c:                   if (add_file_to_cache(path, verbose))
    cache.h:#define add_to_cache(path, st, verbose) add_to_index(&the_index
    cache.h:#define add_file_to_cache(path, verbose) add_file_to_index(&the
    cache.h:extern int add_to_index(struct index_state *, const char *path,
    cache.h:extern int add_file_to_index(struct index_state *, const char *
    read-cache.c:int add_to_index(struct index_state *istate, const char *p
    read-cache.c:int add_file_to_index(struct index_state *istate, const ch
    read-cache.c:   return add_to_index(istate, path, &st, verbose);

Regarding the return value: isn't it very often a bug to ignore them?
Or do you mean to say I should have renamed the function so that old
interface cannot be used accidentally by someone how just knows it
never returns in case of an error?