Re: [PATCH 02/12] wrapper.c: add a new function unlink_or_msg

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Ronnie Sahlberg <sahlberg@xxxxxxxxxx> writes:

> Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx>
> ---
>  git-compat-util.h |  6 ++++++
>  wrapper.c         | 18 ++++++++++++++++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index b6f03b3..426bc98 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -704,12 +704,18 @@ void git_qsort(void *base, size_t nmemb, size_t size,
>  #endif
>  #endif
>  
> +#include "strbuf.h"
> +
>  /*
>   * Preserves errno, prints a message, but gives no warning for ENOENT.
>   * Always returns the return value of unlink(2).
>   */
>  int unlink_or_warn(const char *path);
>  /*
> + * Like unlink_or_warn but populates a strbuf
> + */
> +int unlink_or_msg(const char *file, struct strbuf *err);
> +/*
>   * Likewise for rmdir(2).
>   */
>  int rmdir_or_warn(const char *path);
> diff --git a/wrapper.c b/wrapper.c
> index 740e193..74a0cc0 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -438,6 +438,24 @@ static int warn_if_unremovable(const char *op, const char *file, int rc)
>  	return rc;
>  }
>  
> +int unlink_or_msg(const char *file, struct strbuf *err)
> +{
> +	if (err) {
> +		int rc = unlink(file);
> +		int save_errno = errno;
> +
> +		if (rc < 0 && errno != ENOENT) {
> +			strbuf_addf(err, "unable to unlink %s: %s",
> +				    file, strerror(errno));
> +			errno = save_errno;
> +			return -1;
> +		}
> +		return 0;
> +	}
> +
> +	return unlink_or_warn(file);
> +}

In general, I do not generally like to see messages propagated
upwards from deeper levels of the callchain to the callers to be
used later, primarily because that will easily make it harder to
localize the message-lego.

For this partcular one, shouldn't the caller be doing

	if (unlink(file) && errno != ENOENT) {
        	... do its own error message ...
	}

instead of calling any of the unlink_or_whatever() helper?


>  int unlink_or_warn(const char *file)
>  {
>  	return warn_if_unremovable("unlink", file, unlink(file));
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]