Re: [PATCH v2] builtin/rm.c: use die_errno() when system call fails

[Junio C Hamano <gitster@pobox.com>]

Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com> writes:

> When remove_file_from_index() fails, the errno value contains the
> reason for the failure.

Is that true?

Together with the subject that mentions "system call", I think the
above is not quite right.  Offhand I do not think of any system call
or library call we make, whose failure that sets errnor would cause
the callchain to return an error back to this caller.

Please show us a call graph that originates at this location that
calls remove_file_from_index(), and identify which system/library
call sets errno to cause this function return non-zero.

Otherwise, we would end up feeding an irrelevant garbage value in
error and have die_errno() pick it up.

> Using die() instead of die_errno() loses
> this information, making it harder to diagnose failures. Switch to
> die_errno() to include the system error message in the output.
>
> Signed-off-by: Gatla Vishweshwar Reddy <gatlavishweshwarreddy26@gmail.com>
> ---
>  builtin/rm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 081d0bc375..3f5e6e232d 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -396,7 +396,7 @@ int cmd_rm(int argc,
>  			printf("rm '%s'\n", path);
>  
>  		if (remove_file_from_index(the_repository->index, path))
> -			die(_("git rm: unable to remove %s"), path);
> +			die_errno(_("git rm: unable to remove %s"), path);
>  	}
>  
>  	if (show_only)