Ci lamp dev clean #13
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,12 @@ | |
| #include <stdint.h> | ||
| #include <stdio.h> | ||
| #include <wolfssl/wolfcrypt/coding.h> | ||
|
|
||
| /* Uncomment the following include to compile picotcp-modules in | ||
| * ci_lamp_dev_clean branch on its own and not with tass-connected-device | ||
| * */ | ||
| //#include <wolfcrypt/src/coding.c> | ||
|
There was a problem hiding this comment. @VincentDeHaen Any clue how we can do this less hackerish? The code should work nomatter where we are compiling it from... |
||
|
|
||
| #include "pico_tree.h" | ||
| #include "pico_config.h" | ||
| #include "pico_socket.h" | ||
|
|
@@ -277,6 +283,11 @@ static int8_t pico_http_uri_destroy(struct pico_http_uri *urikey) | |
| PICO_FREE(urikey->host); | ||
| urikey->host = NULL; | ||
| } | ||
| if (urikey->user_pass) | ||
| { | ||
| PICO_FREE(urikey->user_pass); | ||
| urikey->user_pass = NULL; | ||
| } | ||
| PICO_FREE(urikey); | ||
| } | ||
| return HTTP_RETURN_OK; | ||
|
|
@@ -330,7 +341,10 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
| dbg("Start: pico_process_uri(..) %s\n", uri); | ||
| if (!uri || !urikey || uri[0] == '/') | ||
| { | ||
| if (urikey) | ||
| pico_http_uri_destroy(urikey); | ||
| pico_err = PICO_ERR_EINVAL; | ||
| return HTTP_RETURN_ERROR; | ||
| } | ||
| urikey->raw_uri = (char *)PICO_ZALLOC(strlen(uri)+1); | ||
| if (!urikey->raw_uri) | ||
|
|
@@ -358,6 +372,7 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
| /* no protocol specified, assuming by default it's http */ | ||
| urikey->protoHttp = 1; | ||
| } | ||
|
|
||
| /* detect hostname */ | ||
| /*<user>:<password>@<host>:<port>/<url-path>*/ | ||
| /* Check if the user (and password) are specified. */ | ||
|
|
@@ -378,27 +393,22 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
| index++; | ||
|
There was a problem hiding this comment. Phew this function is getting very long. Could you rework this into different smaller functions? (or at least create a jira ticket for it) |
||
| } | ||
|
|
||
| if (userpass_flag){ | ||
|
There was a problem hiding this comment. Can you come up with a different way to parse this. All this index++ stuff makes my head hurt. |
||
| dbg("skipping user name and password to find host\n"); | ||
| // Skip the @ | ||
| index++; | ||
| credentials_index = last_index; | ||
| last_index = index; | ||
| } | ||
| else | ||
| index = last_index; | ||
|
|
||
| dbg("Check hostname format\n"); | ||
| while (uri[index] && uri[index] != '/' && uri[index] != ':') index++; | ||
| if (index == last_index) | ||
| { | ||
| /* wrong format */ | ||
| dbg("wrong format\n"); | ||
| urikey->port = urikey->protoHttp = 0u; | ||
| pico_err = PICO_ERR_EINVAL; | ||
| pico_http_uri_destroy(urikey); | ||
|
|
@@ -425,15 +435,17 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
|
|
||
| if ( userpass_flag ){ | ||
| dbg("extract login and password\n"); | ||
| int inLen = last_index - credentials_index - 1; | ||
| strncpy(buffin, uri + credentials_index, (size_t) inLen); | ||
| buffin[inLen]='\0'; | ||
|
|
||
| if (Base64_Encode((byte*) buffin, (word32) inLen, (byte*) buffout, (word32*) &outLen) != 0){ | ||
| // encoding error | ||
| dbg("error happened while encoding\n"); | ||
| } | ||
| // removing the trailing \n from the base46_Encode | ||
| buffout[strlen(buffout)-2] = '\0'; | ||
|
|
||
| urikey->user_pass = PICO_ZALLOC((uint32_t)(strlen(buffout)+1)); | ||
| if(!urikey->user_pass) | ||
| { | ||
|
|
@@ -443,7 +455,7 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
| return HTTP_RETURN_ERROR; | ||
| } | ||
| memcpy(urikey->user_pass, buffout, (size_t)(strlen(buffout)+1)); | ||
| dbg("processed userpass = %s and lenght = %i\n", urikey->user_pass, (int) strlen(urikey->user_pass)); | ||
| } | ||
|
|
||
| } | ||
|
|
@@ -453,17 +465,27 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
| /* nothing specified */ | ||
| urikey->port = 80u; | ||
| } | ||
| else if (uri[index] == ':' && uri[index+1] && uri[index+1] == '/') { | ||
| // No port after ':' | ||
| dbg("No port after ':' \n"); | ||
| urikey->port = urikey->protoHttp = 0u; | ||
| pico_err = PICO_ERR_EINVAL; | ||
| pico_http_uri_destroy(urikey); | ||
| return HTTP_RETURN_ERROR; | ||
| } | ||
| else if (uri[index] == ':') | ||
| { | ||
| urikey->port = 0u; | ||
| index++; | ||
| while (uri[index] && uri[index] != '/') | ||
| { | ||
| if (!isdigit(uri[index])){ | ||
| dbg("Port is not a number\n"); | ||
| urikey->host = urikey->resource = urikey->user_pass = NULL; | ||
| urikey->port = urikey->protoHttp = 0u; | ||
| pico_http_uri_destroy(urikey); | ||
| return HTTP_RETURN_ERROR; | ||
| } | ||
|
There was a problem hiding this comment. Looking at all of this code, I'd rewrite the whole thing as such: |
||
| urikey->port = (uint16_t)(urikey->port * 10 + (uri[index] - '0')); | ||
| index++; | ||
| } | ||
|
|
@@ -473,7 +495,6 @@ static int8_t pico_process_uri(const char *uri, struct pico_http_uri *urikey) | |
| return pico_process_resource(&uri[index], urikey); | ||
| } | ||
|
|
||
| static int32_t compare_clients(void *ka, void *kb) | ||
| { | ||
| return ((struct pico_http_client *)ka)->connectionID - ((struct pico_http_client *)kb)->connectionID; | ||
|
|
@@ -588,8 +609,8 @@ static void treat_read_event(struct pico_http_client *client) | |
| } | ||
| else | ||
| { | ||
| /* just let the user know that data has arrived, if chunked data comes, | ||
| * will be treated in the read api. */ | ||
| client->wakeup(EV_HTTP_BODY, client->connectionID); | ||
| } | ||
| } | ||
|
|
@@ -1320,14 +1341,7 @@ static int32_t client_open(char *uri, void (*wakeup)(uint16_t ev, uint16_t conn) | |
| } | ||
|
|
||
| client->wakeup = wakeup; | ||
| client->connectionID = connID >= 0 ? connID : global_client_conn_ID++; | ||
|
There was a problem hiding this comment. Here brackets would improve readability. Just to verify: you know that global_client_conn_ID is only incremeted AFTER it's assigned? That's ok? |
||
|
|
||
| client->urikey = PICO_ZALLOC(sizeof(struct pico_http_uri)); | ||
|
|
||
|
|
@@ -1895,15 +1909,15 @@ static inline int32_t read_chunked_data(struct pico_http_client *client, unsigne | |
| { | ||
| uint32_t len_read = 0; | ||
| int32_t value; | ||
| // read the chunk line | ||
|
There was a problem hiding this comment. I think these comments were actually done on purpose. The old comments are the actual proper C-style comments |
||
| if (read_chunk_line(client) == HTTP_RETURN_ERROR) | ||
| { | ||
| dbg("Probably the chunk is malformed or parsed wrong...\n"); | ||
| client->wakeup(EV_HTTP_ERROR, client->connectionID); | ||
| return HTTP_RETURN_ERROR; | ||
| } | ||
|
|
||
| // nothing to read, no use to try | ||
| if (client->state != HTTP_READING_BODY) | ||
| { | ||
| pico_err = PICO_ERR_EAGAIN; | ||
|
|
@@ -1944,7 +1958,7 @@ int32_t MOCKABLE pico_http_client_read_body(uint16_t conn, unsigned char *data, | |
| } | ||
| if (client->header->transfer_coding == HTTP_TRANSFER_FULL) | ||
| { | ||
| //check to make sure we don't read more than the header told us, content-length | ||
| if ((client->header->content_length_or_chunk - client->body_read) < size) | ||
| { | ||
| size = client->header->content_length_or_chunk; | ||
|
|
@@ -1959,7 +1973,11 @@ int32_t MOCKABLE pico_http_client_read_body(uint16_t conn, unsigned char *data, | |
| } | ||
| else | ||
| { | ||
| /* | ||
| * client->state will be set to HTTP_READ_BODY_DONE if we reach the | ||
| * ending '0' at the end of the body, read_chunked_data make sure | ||
| * we don't read to mutch. | ||
| */ | ||
| client->body_read += bytes_read; | ||
| bytes_read = read_chunked_data(client, data, size); | ||
| } | ||
|
|
@@ -2384,7 +2402,7 @@ int8_t pico_http_set_close_ev(uint16_t conn) | |
| return HTTP_RETURN_ERROR; | ||
| } | ||
| client->wakeup(EV_HTTP_CLOSE, client->connectionID); | ||
| return EV_HTTP_CLOSE; | ||
|
There was a problem hiding this comment. what's the impact of this line? There was a problem hiding this comment. Getting rid of warning. |
||
| } | ||
|
|
||
|
|
||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
@frederikvs Do we usually get the header files from the out directory ? Although, with the picotcp one it seems that we're also retrieving them from the build directory...