New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle the schema registry url ending with '/' #38
Conversation
src/rest.c
Outdated
* the last redundant '/' in the url. | ||
*/ | ||
url_len = strlen(ul->urls[ul->idx]); | ||
if (url_len > 0 && ul->urls[ul->idx][url_len - 1] == '/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggest removing all trailing slashes:
while (url_len > 0 && ..urls[url_len-1] == '/')
url_len--;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion.
src/rest.c
Outdated
url_len = strlen(ul->urls[ul->idx]); | ||
if (url_len > 0 && ul->urls[ul->idx][url_len - 1] == '/') | ||
url_len --; | ||
snprintf(tmpurl, url_len + 1, "%s", ul->urls[ul->idx]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simplified by passing the string length:
sprintf(tmpurl, "%.*s%s", (int)url_len, ul->urls[ul->idx], url_path);
src/rest.c
Outdated
@@ -372,6 +372,7 @@ static rest_response_t *rest_req (url_list_t *ul, rest_cmd_t cmd, | |||
int url_path_len; | |||
va_list ap2; | |||
const int debug = 0; | |||
size_t url_len; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move this down to the blcok where it is used (the do { block)
src/rest.c
Outdated
@@ -433,17 +432,17 @@ static rest_response_t *rest_req (url_list_t *ul, rest_cmd_t cmd, | |||
ccode = CURLE_URL_MALFORMAT; | |||
tmpurl = alloca(ul->max_len + 1 + strlen(url_path) + 1); | |||
start_idx = ul->idx; | |||
size_t url_len; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should go at the beginning of the do {
block:
do {
size_t url_len = strlen(...);
src/rest.c
Outdated
url_len --; | ||
snprintf(tmpurl, url_len + 1, "%s", ul->urls[ul->idx]); | ||
sprintf(tmpurl + url_len, "%s", url_path); | ||
sprintf(tmpurl, "%.*s%s", url_len, ul->urls[ul->idx], url_path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to cast url_len to int
src/rest.c
Outdated
@@ -433,10 +433,18 @@ static rest_response_t *rest_req (url_list_t *ul, rest_cmd_t cmd, | |||
tmpurl = alloca(ul->max_len + 1 + strlen(url_path) + 1); | |||
start_idx = ul->idx; | |||
do { | |||
sprintf(tmpurl, "%s%s", ul->urls[ul->idx], url_path); | |||
/* Hanld the '/' in url |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "Handle"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
[clabot:check] |
@confluentinc It looks like @fanfuxiaoran just signed our Contributor License Agreement. 👍 Always at your service, clabot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you! |
No description provided.