[PATCHv2 0/4] Implements the GIT_TRACE_CURL environment variable

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCHv2 0/4] Implements the GIT_TRACE_CURL environment variable

Elia Pinto
This is the second version but in reality is the complete rewriting of the patches discussed here

$gmane/290520
$gmane/290521

Changes from V1

- introduced GIT_TRACE_CURL variable with its documentation
- changed the name of the temporary variable "i" in "w" in the helper routine
- used the c escape sequences instead of the hex equivalent
- dropped the previous GIT_DEBUG_CURL env var
- curl_dump and curl_trace factored out to a shared implementation
in http.c

 
Elia Pinto (4):
  http.h: Add debug callback and helper routine for implementing     the
    GIT_TRACE_CURL environment variable in http.c
  http.c: implements the GIT_TRACE_CURL environment variable
  git.txt: document the new GIT_TRACE_CURL environment variable
  imap-send.c: introduce the GIT_TRACE_CURL enviroment variable

 Documentation/git.txt |  8 +++++
 http.c                | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 http.h                |  6 ++++
 imap-send.c           |  6 ++++
 4 files changed, 117 insertions(+), 1 deletion(-)

--
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4] http.h: Add debug callback and helper routine for implementing the GIT_TRACE_CURL environment variable in http.c

Elia Pinto
Add the debug callback and helper routine prototype used by
curl_easy_setopt CURLOPT_DEBUGFUNCTION in http.c
for implementing the GIT_TRACE_CURL environment variable


Helped-by: Torsten Bögershausen <[hidden email]>
Helped-by: Ramsay Jones <[hidden email]>
Helped-by: Junio C Hamano <[hidden email]>
Helped-by: Eric Sunshine <[hidden email]>
Helped-by: Jeff King <[hidden email]>
Signed-off-by: Elia Pinto <[hidden email]>
---
 http.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/http.h b/http.h
index 4ef4bbd..a2d10bc 100644
--- a/http.h
+++ b/http.h
@@ -224,4 +224,10 @@ extern int finish_http_object_request(struct http_object_request *freq);
 extern void abort_http_object_request(struct http_object_request *freq);
 extern void release_http_object_request(struct http_object_request *freq);
 
+/* Debug callback and helper routine for curl_easy_setopt CURLOPT_DEBUGFUNCTION */
+static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
+int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp);
+void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex);
+
+
 #endif /* HTTP_H */
--
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4] http.c: implements the GIT_TRACE_CURL environment variable

Elia Pinto
In reply to this post by Elia Pinto
Implements the GIT_TRACE_CURL environment variable to allow a
greater degree of detail of GIT_CURL_VERBOSE, in particular
the complete transport header and all the data payload exchanged.
It might be useful if a particular situation could require a more
thorough debugging analysis


Helped-by: Torsten Bögershausen <[hidden email]>
Helped-by: Ramsay Jones <[hidden email]>
Helped-by: Junio C Hamano <[hidden email]>
Helped-by: Eric Sunshine <[hidden email]>
Helped-by: Jeff King <[hidden email]>
Signed-off-by: Elia Pinto <[hidden email]>
---
 http.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/http.c b/http.c
index 4304b80..278991e 100644
--- a/http.c
+++ b/http.c
@@ -11,6 +11,9 @@
 #include "gettext.h"
 #include "transport.h"
 
+/*
+tatic struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
+*/
 #if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
 #else
@@ -464,6 +467,95 @@ static void set_curl_keepalive(CURL *c)
 }
 #endif
 
+
+void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
+{
+ size_t i;
+ size_t w;
+ struct strbuf out = STRBUF_INIT;;
+
+ unsigned int width = 0x10;
+
+ if (nohex)
+ /* without the hex output, we can fit more on screen */
+ width = 0x40;
+
+ strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
+ text, (long)size, (long)size);
+
+ for (i = 0; i < size; i += width) {
+
+ strbuf_addf(&out, "%4.4lx: ", (long)i);
+
+ if (!nohex) {
+ /* hex not disabled, show it */
+ for (w = 0; w < width; w++)
+ if (i + w < size)
+ strbuf_addf(&out, "%02x ", ptr[i + w]);
+ else
+ strbuf_addf(&out,"   ");
+ }
+
+ for (w = 0; (w < width) && (i + w < size); w++) {
+ /* check for 0D0A; if found, skip past and start a new line of output */
+ if (nohex && (i + w + 1 < size) && ptr[i + w] == '\r'
+    && ptr[i + w + 1] == '\n') {
+ i += (w + 2 - width);
+ break;
+ }
+ strbuf_addch(&out, (ptr[i + w] >= 0x20)
+ && (ptr[i + w] < 0x80) ? ptr[i + w] : '.');
+ /* check again for 0D0A, to avoid an extra \n if it's at width */
+ if (nohex && (i + w + 2 < size)
+    && ptr[i + w + 1] == '\r'
+    && ptr[i + w + 2] == '\n') {
+ i += (w + 3 - width);
+ break;
+ }
+ }
+ strbuf_addch(&out, '\n');
+ trace_strbuf(&trace_curl, &out);
+ strbuf_release(&out);
+ }
+}
+
+int curl_trace(CURL *handle, curl_infotype type,
+     char *data, size_t size, void *userp)
+{
+ const char *text;
+ (void)handle; /* prevent compiler warning */
+
+ switch (type) {
+ case CURLINFO_TEXT:
+ trace_printf_key(&trace_curl, "== Info: %s", data);
+ default: /* in case a new one is introduced to shock us */
+ return 0;
+
+ case CURLINFO_HEADER_OUT:
+ text = "=> Send header";
+ break;
+ case CURLINFO_DATA_OUT:
+ text = "=> Send data";
+ break;
+ case CURLINFO_SSL_DATA_OUT:
+ text = "=> Send SSL data";
+ break;
+ case CURLINFO_HEADER_IN:
+ text = "<= Recv header";
+ break;
+ case CURLINFO_DATA_IN:
+ text = "<= Recv data";
+ break;
+ case CURLINFO_SSL_DATA_IN:
+ text = "<= Recv SSL data";
+ break;
+ }
+
+ curl_dump(text, (unsigned char *)data, size, 1);
+ return 0;
+}
+
+
 static CURL *get_curl_handle(void)
 {
  CURL *result = curl_easy_init();
@@ -563,7 +655,11 @@ static CURL *get_curl_handle(void)
  "your curl version is too old (>= 7.19.4)");
 #endif
 
- if (getenv("GIT_CURL_VERBOSE"))
+ if (trace_want(&trace_curl)) {
+ curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
+ curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
+ curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
+ } else if (getenv("GIT_CURL_VERBOSE"))
  curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
 
  curl_easy_setopt(result, CURLOPT_USERAGENT,
--
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4] git.txt: document the new GIT_TRACE_CURL environment variable

Elia Pinto
In reply to this post by Elia Pinto
Describe the purpose of the GIT_TRACE_CURL environment variable

Helped-by: Torsten Bögershausen <[hidden email]>
Helped-by: Ramsay Jones <[hidden email]>
Helped-by: Junio C Hamano <[hidden email]>
Helped-by: Eric Sunshine <[hidden email]>
Helped-by: Jeff King <[hidden email]>
Signed-off-by: Elia Pinto <[hidden email]>
---
 Documentation/git.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 8afe349..958db0f 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -1075,6 +1075,14 @@ of clones and fetches.
  cloning of shallow repositories.
  See 'GIT_TRACE' for available trace output options.
 
+'GIT_TRACE_CURL'::
+ Enables a curl full trace dump of all incoming and outgoing data,
+ including descriptive information, of the git transport protocol.
+ This is similar to doing curl --trace-ascii on the command line.
+ This option overrides setting the GIT_CURL_VERBOSE environment
+ variable.
+ See 'GIT_TRACE' for available trace output options.
+
 'GIT_LITERAL_PATHSPECS'::
  Setting this variable to `1` will cause Git to treat all
  pathspecs literally, rather than as glob patterns. For example,
--
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4] imap-send.c: introduce the GIT_TRACE_CURL enviroment variable

Elia Pinto
In reply to this post by Elia Pinto
Permit the use of the GIT_TRACE_CURL environment variable calling
the curl_trace and curl_dump http.c helper routine


Helped-by: Torsten Bögershausen <[hidden email]>
Helped-by: Ramsay Jones <[hidden email]>
Helped-by: Junio C Hamano <[hidden email]>
Helped-by: Eric Sunshine <[hidden email]>
Helped-by: Jeff King <[hidden email]>
Signed-off-by: Elia Pinto <[hidden email]>
---
 imap-send.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/imap-send.c b/imap-send.c
index 938c691..b371a78 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1444,6 +1444,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
  if (0 < verbosity || getenv("GIT_CURL_VERBOSE"))
  curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
 
+ if (trace_want(&trace_curl)) {
+ curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
+ curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, curl_trace);
+ curl_easy_setopt(curl, CURLOPT_DEBUGDATA, NULL);
+ }
+
  return curl;
 }
 
--
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/4] http.h: Add debug callback and helper routine for implementing the GIT_TRACE_CURL environment variable in http.c

Junio C Hamano
In reply to this post by Elia Pinto
Elia Pinto <[hidden email]> writes:

> Add the debug callback and helper routine prototype used by
> curl_easy_setopt CURLOPT_DEBUGFUNCTION in http.c
> for implementing the GIT_TRACE_CURL environment variable
>
>
> Helped-by: Torsten Bögershausen <[hidden email]>
> Helped-by: Ramsay Jones <[hidden email]>
> Helped-by: Junio C Hamano <[hidden email]>
> Helped-by: Eric Sunshine <[hidden email]>
> Helped-by: Jeff King <[hidden email]>
> Signed-off-by: Elia Pinto <[hidden email]>
> ---
>  http.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/http.h b/http.h
> index 4ef4bbd..a2d10bc 100644
> --- a/http.h
> +++ b/http.h
> @@ -224,4 +224,10 @@ extern int finish_http_object_request(struct http_object_request *freq);
>  extern void abort_http_object_request(struct http_object_request *freq);
>  extern void release_http_object_request(struct http_object_request *freq);
>  
> +/* Debug callback and helper routine for curl_easy_setopt CURLOPT_DEBUGFUNCTION */
> +static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);

This will be multiply instanciated in every *.c file that includes http.h?

> +int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp);
> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex);
> +
> +
>  #endif /* HTTP_H */

In any case, you'd want to combine this with 2/4 and write a single
log message for the combined change.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/4] http.c: implements the GIT_TRACE_CURL environment variable

Junio C Hamano
In reply to this post by Elia Pinto
Elia Pinto <[hidden email]> writes:

> Implements the GIT_TRACE_CURL environment variable to allow a
> greater degree of detail of GIT_CURL_VERBOSE, in particular
> the complete transport header and all the data payload exchanged.
> It might be useful if a particular situation could require a more
> thorough debugging analysis
>
>
> Helped-by: Torsten Bögershausen <[hidden email]>
> Helped-by: Ramsay Jones <[hidden email]>
> Helped-by: Junio C Hamano <[hidden email]>
> Helped-by: Eric Sunshine <[hidden email]>
> Helped-by: Jeff King <[hidden email]>
> Signed-off-by: Elia Pinto <[hidden email]>
> ---
>  http.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/http.c b/http.c
> index 4304b80..278991e 100644
> --- a/http.c
> +++ b/http.c
> @@ -11,6 +11,9 @@
>  #include "gettext.h"
>  #include "transport.h"
>  
> +/*
> +tatic struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
> +*/

Is this a sign that this hasn't been proof-read before sending?

>  #if LIBCURL_VERSION_NUM >= 0x070a08
>  long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
>  #else
> @@ -464,6 +467,95 @@ static void set_curl_keepalive(CURL *c)
>  }
>  #endif
>  
> +
> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
> +{
> + size_t i;
> + size_t w;
> + struct strbuf out = STRBUF_INIT;;
> +
> + unsigned int width = 0x10;
> +
> + if (nohex)
> + /* without the hex output, we can fit more on screen */
> + width = 0x40;
> +
> + strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
> + text, (long)size, (long)size);
> +
> + for (i = 0; i < size; i += width) {
> +
> + strbuf_addf(&out, "%4.4lx: ", (long)i);
> +
> + if (!nohex) {
> + /* hex not disabled, show it */
> + for (w = 0; w < width; w++)
> + if (i + w < size)
> + strbuf_addf(&out, "%02x ", ptr[i + w]);
> + else
> + strbuf_addf(&out,"   ");
> + }
> +
> + for (w = 0; (w < width) && (i + w < size); w++) {
> + /* check for 0D0A; if found, skip past and start a new line of output */
> + if (nohex && (i + w + 1 < size) && ptr[i + w] == '\r'
> +    && ptr[i + w + 1] == '\n') {
> + i += (w + 2 - width);
> + break;
> + }
> + strbuf_addch(&out, (ptr[i + w] >= 0x20)
> + && (ptr[i + w] < 0x80) ? ptr[i + w] : '.');
> + /* check again for 0D0A, to avoid an extra \n if it's at width */
> + if (nohex && (i + w + 2 < size)
> +    && ptr[i + w + 1] == '\r'
> +    && ptr[i + w + 2] == '\n') {
> + i += (w + 3 - width);
> + break;
> + }
> + }
> + strbuf_addch(&out, '\n');
> + trace_strbuf(&trace_curl, &out);
> + strbuf_release(&out);
> + }
> +}
> +
> +int curl_trace(CURL *handle, curl_infotype type,
> +     char *data, size_t size, void *userp)
> +{
> + const char *text;
> + (void)handle; /* prevent compiler warning */
> +
> + switch (type) {
> + case CURLINFO_TEXT:
> + trace_printf_key(&trace_curl, "== Info: %s", data);
> + default: /* in case a new one is introduced to shock us */
> + return 0;
> +
> + case CURLINFO_HEADER_OUT:
> + text = "=> Send header";
> + break;
> + case CURLINFO_DATA_OUT:
> + text = "=> Send data";
> + break;
> + case CURLINFO_SSL_DATA_OUT:
> + text = "=> Send SSL data";
> + break;
> + case CURLINFO_HEADER_IN:
> + text = "<= Recv header";
> + break;
> + case CURLINFO_DATA_IN:
> + text = "<= Recv data";
> + break;
> + case CURLINFO_SSL_DATA_IN:
> + text = "<= Recv SSL data";
> + break;
> + }
> +
> + curl_dump(text, (unsigned char *)data, size, 1);
> + return 0;
> +}
> +
> +
>  static CURL *get_curl_handle(void)
>  {
>   CURL *result = curl_easy_init();
> @@ -563,7 +655,11 @@ static CURL *get_curl_handle(void)
>   "your curl version is too old (>= 7.19.4)");
>  #endif
>  
> - if (getenv("GIT_CURL_VERBOSE"))
> + if (trace_want(&trace_curl)) {
> + curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
> + curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
> + curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
> + } else if (getenv("GIT_CURL_VERBOSE"))
>   curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>  
>   curl_easy_setopt(result, CURLOPT_USERAGENT,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/4] imap-send.c: introduce the GIT_TRACE_CURL enviroment variable

Junio C Hamano
In reply to this post by Elia Pinto
Elia Pinto <[hidden email]> writes:

> Permit the use of the GIT_TRACE_CURL environment variable calling
> the curl_trace and curl_dump http.c helper routine

s/$/./; the patch itself is very concise and the "dump" thing in 3/4
looked sensible.

>
> Helped-by: Torsten Bögershausen <[hidden email]>
> Helped-by: Ramsay Jones <[hidden email]>
> Helped-by: Junio C Hamano <[hidden email]>
> Helped-by: Eric Sunshine <[hidden email]>
> Helped-by: Jeff King <[hidden email]>
> Signed-off-by: Elia Pinto <[hidden email]>
> ---
>  imap-send.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/imap-send.c b/imap-send.c
> index 938c691..b371a78 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1444,6 +1444,12 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
>   if (0 < verbosity || getenv("GIT_CURL_VERBOSE"))
>   curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
>  
> + if (trace_want(&trace_curl)) {
> + curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
> + curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, curl_trace);
> + curl_easy_setopt(curl, CURLOPT_DEBUGDATA, NULL);
> + }
> +
>   return curl;
>  }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/4] http.c: implements the GIT_TRACE_CURL environment variable

Junio C Hamano
In reply to this post by Elia Pinto
Elia Pinto <[hidden email]> writes:

> Implements the GIT_TRACE_CURL environment variable to allow a
> greater degree of detail of GIT_CURL_VERBOSE, in particular
> the complete transport header and all the data payload exchanged.
> It might be useful if a particular situation could require a more
> thorough debugging analysis
>
>
> Helped-by: Torsten Bögershausen <[hidden email]>
> Helped-by: Ramsay Jones <[hidden email]>
> Helped-by: Junio C Hamano <[hidden email]>
> Helped-by: Eric Sunshine <[hidden email]>
> Helped-by: Jeff King <[hidden email]>
> Signed-off-by: Elia Pinto <[hidden email]>
> ---
>  http.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 97 insertions(+), 1 deletion(-)
>
> diff --git a/http.c b/http.c
> index 4304b80..278991e 100644
> --- a/http.c
> +++ b/http.c
> @@ -11,6 +11,9 @@
>  #include "gettext.h"
>  #include "transport.h"
>  
> +/*
> +tatic struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
> +*/
>  #if LIBCURL_VERSION_NUM >= 0x070a08
>  long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
>  #else
> @@ -464,6 +467,95 @@ static void set_curl_keepalive(CURL *c)
>  }
>  #endif
>  
> +
> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
> +{
> + size_t i;
> + size_t w;
> + struct strbuf out = STRBUF_INIT;;

;; makes the next line decl-after-stmt.

> +
> + unsigned int width = 0x10;




> +
> + if (nohex)
> + /* without the hex output, we can fit more on screen */
> + width = 0x40;
> +
> + strbuf_addf(&out, "%s, %10.10ld bytes (0x%8.8lx)\n",
> + text, (long)size, (long)size);
> +
> + for (i = 0; i < size; i += width) {
> +
> + strbuf_addf(&out, "%4.4lx: ", (long)i);
> +
> + if (!nohex) {
> + /* hex not disabled, show it */
> + for (w = 0; w < width; w++)
> + if (i + w < size)
> + strbuf_addf(&out, "%02x ", ptr[i + w]);
> + else
> + strbuf_addf(&out,"   ");
> + }
> +
> + for (w = 0; (w < width) && (i + w < size); w++) {
> + /* check for 0D0A; if found, skip past and start a new line of output */
> + if (nohex && (i + w + 1 < size) && ptr[i + w] == '\r'
> +    && ptr[i + w + 1] == '\n') {
> + i += (w + 2 - width);
> + break;
> + }
> + strbuf_addch(&out, (ptr[i + w] >= 0x20)
> + && (ptr[i + w] < 0x80) ? ptr[i + w] : '.');
> + /* check again for 0D0A, to avoid an extra \n if it's at width */
> + if (nohex && (i + w + 2 < size)
> +    && ptr[i + w + 1] == '\r'
> +    && ptr[i + w + 2] == '\n') {
> + i += (w + 3 - width);
> + break;
> + }
> + }
> + strbuf_addch(&out, '\n');
> + trace_strbuf(&trace_curl, &out);
> + strbuf_release(&out);
> + }
> +}
> +
> +int curl_trace(CURL *handle, curl_infotype type,
> +     char *data, size_t size, void *userp)
> +{
> + const char *text;
> + (void)handle; /* prevent compiler warning */
> +
> + switch (type) {
> + case CURLINFO_TEXT:
> + trace_printf_key(&trace_curl, "== Info: %s", data);
> + default: /* in case a new one is introduced to shock us */
> + return 0;
> +
> + case CURLINFO_HEADER_OUT:
> + text = "=> Send header";
> + break;
> + case CURLINFO_DATA_OUT:
> + text = "=> Send data";
> + break;
> + case CURLINFO_SSL_DATA_OUT:
> + text = "=> Send SSL data";
> + break;
> + case CURLINFO_HEADER_IN:
> + text = "<= Recv header";
> + break;
> + case CURLINFO_DATA_IN:
> + text = "<= Recv data";
> + break;
> + case CURLINFO_SSL_DATA_IN:
> + text = "<= Recv SSL data";
> + break;
> + }
> +
> + curl_dump(text, (unsigned char *)data, size, 1);
> + return 0;
> +}
> +
> +
>  static CURL *get_curl_handle(void)
>  {
>   CURL *result = curl_easy_init();
> @@ -563,7 +655,11 @@ static CURL *get_curl_handle(void)
>   "your curl version is too old (>= 7.19.4)");
>  #endif
>  
> - if (getenv("GIT_CURL_VERBOSE"))
> + if (trace_want(&trace_curl)) {
> + curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
> + curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
> + curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
> + } else if (getenv("GIT_CURL_VERBOSE"))
>   curl_easy_setopt(result, CURLOPT_VERBOSE, 1);
>  
>   curl_easy_setopt(result, CURLOPT_USERAGENT,
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/4] http.h: Add debug callback and helper routine for implementing the GIT_TRACE_CURL environment variable in http.c

Ramsay Jones-2
In reply to this post by Elia Pinto


On 19/04/16 16:10, Elia Pinto wrote:

> Add the debug callback and helper routine prototype used by
> curl_easy_setopt CURLOPT_DEBUGFUNCTION in http.c
> for implementing the GIT_TRACE_CURL environment variable
>
>
> Helped-by: Torsten Bögershausen <[hidden email]>
> Helped-by: Ramsay Jones <[hidden email]>
> Helped-by: Junio C Hamano <[hidden email]>
> Helped-by: Eric Sunshine <[hidden email]>
> Helped-by: Jeff King <[hidden email]>
> Signed-off-by: Elia Pinto <[hidden email]>
> ---
>  http.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/http.h b/http.h
> index 4ef4bbd..a2d10bc 100644
> --- a/http.h
> +++ b/http.h
> @@ -224,4 +224,10 @@ extern int finish_http_object_request(struct http_object_request *freq);
>  extern void abort_http_object_request(struct http_object_request *freq);
>  extern void release_http_object_request(struct http_object_request *freq);
>  
> +/* Debug callback and helper routine for curl_easy_setopt CURLOPT_DEBUGFUNCTION */
> +static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);

Ah no, this would add 6 instances of the 'trace_curl' key in http-fetch.c,
http-push.c, http-walker.c, http.c, imap-send.c and remote-curl.c. Hmm ...
since these would end up in different executables (by and large) it might
work OK, ... but is simply not necessary.

Also, patches #1 and #2 should be squashed into one patch and, since the
curl_dump() function is only called from http.c, it can be a static symbol.

I think the minimal fixup (including Junio's comment on patch #2, which also
triggered for me) is given in the patch below.

Hope that helps.

ATB,
Ramsay Jones

> +int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp);
> +void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex);
> +
> +
>  #endif /* HTTP_H */

-- >8 --
Subject: [PATCH] curl-trace: fix scope/visibility of various symbols

Signed-off-by: Ramsay Jones <[hidden email]>
---
 http.c | 9 +++------
 http.h | 3 +--
 2 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/http.c b/http.c
index 64dd975..ce91421 100644
--- a/http.c
+++ b/http.c
@@ -11,9 +11,7 @@
 #include "gettext.h"
 #include "transport.h"
 
-/*
-tatic struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
-*/
+struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
 #if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
 #else
@@ -468,12 +466,11 @@ static void set_curl_keepalive(CURL *c)
 #endif
 
 
-void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
+static void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex)
 {
  size_t i;
  size_t w;
- struct strbuf out = STRBUF_INIT;;
-
+ struct strbuf out = STRBUF_INIT;
  unsigned int width = 0x10;
 
  if (nohex)
diff --git a/http.h b/http.h
index a2d10bc..00e4ad7 100644
--- a/http.h
+++ b/http.h
@@ -225,9 +225,8 @@ extern void abort_http_object_request(struct http_object_request *freq);
 extern void release_http_object_request(struct http_object_request *freq);
 
 /* Debug callback and helper routine for curl_easy_setopt CURLOPT_DEBUGFUNCTION */
-static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
+extern struct trace_key trace_curl;
 int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp);
-void curl_dump(const char *text, unsigned char *ptr, size_t size, char nohex);
 
 
 #endif /* HTTP_H */
--
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/4] http.h: Add debug callback and helper routine for implementing the GIT_TRACE_CURL environment variable in http.c

Ramsay Jones-2


On 19/04/16 22:48, Ramsay Jones wrote:
>
[snip]

> I think the minimal fixup (including Junio's comment on patch #2, which also
> triggered for me) is given in the patch below.

BTW, if you want to have a single static instance of the 'struct trace_key',
then the following patch on top should work. (Also, I have compiled and run
the testsuite on these patches, but _not_ tested that the trace functionality
actually works!) ;-)

HTH

ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] curl-trace: reduce visibility of the trace key struct

Signed-off-by: Ramsay Jones <[hidden email]>
---
 http.c      | 9 +++++++--
 http.h      | 2 +-
 imap-send.c | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/http.c b/http.c
index ce91421..eed6dba 100644
--- a/http.c
+++ b/http.c
@@ -11,7 +11,7 @@
 #include "gettext.h"
 #include "transport.h"
 
-struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
+static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
 #if LIBCURL_VERSION_NUM >= 0x070a08
 long int git_curl_ipresolve = CURL_IPRESOLVE_WHATEVER;
 #else
@@ -516,6 +516,11 @@ static void curl_dump(const char *text, unsigned char *ptr, size_t size, char no
  }
 }
 
+int curl_trace_want(void)
+{
+ return trace_want(&trace_curl);
+}
+
 int curl_trace(CURL *handle, curl_infotype type,
      char *data, size_t size, void *userp)
 {
@@ -652,7 +657,7 @@ static CURL *get_curl_handle(void)
  "your curl version is too old (>= 7.19.4)");
 #endif
 
- if (trace_want(&trace_curl)) {
+ if (curl_trace_want()) {
  curl_easy_setopt(result, CURLOPT_VERBOSE, 1L);
  curl_easy_setopt(result, CURLOPT_DEBUGFUNCTION, curl_trace);
  curl_easy_setopt(result, CURLOPT_DEBUGDATA, NULL);
diff --git a/http.h b/http.h
index 00e4ad7..153ff17 100644
--- a/http.h
+++ b/http.h
@@ -225,7 +225,7 @@ extern void abort_http_object_request(struct http_object_request *freq);
 extern void release_http_object_request(struct http_object_request *freq);
 
 /* Debug callback and helper routine for curl_easy_setopt CURLOPT_DEBUGFUNCTION */
-extern struct trace_key trace_curl;
+int curl_trace_want(void);
 int curl_trace(CURL *handle, curl_infotype type, char *data, size_t size, void *userp);
 
 
diff --git a/imap-send.c b/imap-send.c
index 36ff843..627ffa4 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1444,7 +1444,7 @@ static CURL *setup_curl(struct imap_server_conf *srvc)
  if (0 < verbosity || getenv("GIT_CURL_VERBOSE"))
  curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
 
- if (trace_want(&trace_curl)) {
+ if (curl_trace_want()) {
  curl_easy_setopt(curl, CURLOPT_VERBOSE, 1L);
  curl_easy_setopt(curl, CURLOPT_DEBUGFUNCTION, curl_trace);
  curl_easy_setopt(curl, CURLOPT_DEBUGDATA, NULL);
--
2.8.0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html