Re: [PATCH] push: anonymize URLs in error messages and warnings

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

 



"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:

>  builtin/push.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Is this something we can protect with a test?  perhaps like 882d49ca
(push: anonymize URL in status output, 2016-07-13) did?

> diff --git a/builtin/push.c b/builtin/push.c
> index 6dbf0f0bb71..bd2a2cbfbd7 100644
> --- a/builtin/push.c
> +++ b/builtin/push.c
> @@ -340,6 +340,7 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
>  {
>  	int err;
>  	unsigned int reject_reasons;
> +	char *anon_url = transport_anonymize_url(transport->url);

Do we need to anonymize this early, way before we know we'd fail the
push?  It wouldn't be that transport->url is going to be munged
before we realize that we have to issue an error message---otherwise
you'd not be writing a patch to replace transport->url in the error
message.  So this placement of anonymize call sounds like taking the
error path as the main flow of control.

In other words, would it break to squash the following change in?

 builtin/push.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index bd2a2cbfbd..b88948b07e 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -340,7 +340,6 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 {
 	int err;
 	unsigned int reject_reasons;
-	char *anon_url = transport_anonymize_url(transport->url);
 
 	transport_set_verbosity(transport, verbosity, progress);
 	transport->family = family;
@@ -364,13 +363,14 @@ static int push_with_options(struct transport *transport, struct refspec *rs,
 			     rs, flags, &reject_reasons);
 	trace2_region_leave("push", "transport_push", the_repository);
 	if (err != 0) {
+		char *anon_url = transport_anonymize_url(transport->url);
 		fprintf(stderr, "%s", push_get_color(PUSH_COLOR_ERROR));
 		error(_("failed to push some refs to '%s'"), anon_url);
 		fprintf(stderr, "%s", push_get_color(PUSH_COLOR_RESET));
+		free(anon_url);
 	}
 
 	err |= transport_disconnect(transport);
-	free(anon_url);
 	if (!err)
 		return 0;
 



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

  Powered by Linux