Re: [PATCH v4 1/2] usage: Introduce error_errno corresponding to die_errno

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

 



Ramkumar Ramachandra <artagnon@xxxxxxxxx> writes:

> diff --git a/git-compat-util.h b/git-compat-util.h
> index 40498b3..7b82038 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -241,6 +241,7 @@ extern NORETURN void usagef(const char *err, ...) __attribute__((format (printf,
>  extern NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
> +extern int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2)));
>  extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));

Anybody remotely familiar with die_errno() would at least expect that the
new and improved error_errno() function would give the errno neatly
formatted via strerror() in the message, but that is not what the function
does.

> diff --git a/usage.c b/usage.c
> index b5e67e3..c89efcc 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -107,6 +107,16 @@ int error(const char *err, ...)
>  	return -1;
>  }
>  
> +int error_errno(const char *err, ...)
> +{
> +	va_list params;
> +
> +	va_start(params, err);
> +	error_routine(err, params);
> +	va_end(params);
> +	return -errno;
> +}

Can error_routine() do something to cause errno to change?

For that matter, I suspect that the caller who would _care_ about what
kind of error it got would need to save it away itself anyway.  For
example, it is not entirely clear if the callsite of this function in your
other patch is correct.  Can rollback_lock_file() cause errno to change?

	git_config(git_rerere_gc_config, NULL);
	dir = opendir(git_path("rr-cache"));
	if (!dir) {
+		int err = errno;
-		rollback_lock_file(&write_lock);
-		return error_errno("Unable to open rr-cache directory");
+		error("Untable to open rr-cache directory: %s", strerror(err));
+		return -err;
	}

I would prefer to do without introducing a confusingly named API function
whose first and only usecase does not even suggest it would be useful.
--
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]