You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/05/14 00:35:40 UTC
[GitHub] [incubator-nuttx-apps] mlyszczek commented on a change in pull request #695: webclient: Implement non-blocking I/O
mlyszczek commented on a change in pull request #695:
URL: https://github.com/apache/incubator-nuttx-apps/pull/695#discussion_r632183250
##########
File path: netutils/webclient/webclient.c
##########
@@ -910,131 +1040,184 @@ int webclient_perform(FAR struct webclient_context *ctx)
goto errout_with_errno;
}
- ret = conn_send_all(ctx, &conn, input_buffer,
- input_buffer_size);
- if (ret < 0)
+ ninfo("Got %zu bytes body chunk / total remaining %zu bytes\n",
+ input_buffer_size, ws->state_len);
+ ws->data_buffer = input_buffer;
+ ws->data_len = input_buffer_size;
+ ws->state_offset = 0;
+ }
+ else
+ {
+ size_t bytes_to_send = ws->data_len - ws->state_offset;
+
+ DEBUGASSERT(bytes_to_send <= ws->state_len);
+ ssize_t ssz = conn_send(ctx, conn,
+ ws->data_buffer + ws->state_offset,
+ bytes_to_send);
+ if (ssz < 0)
{
- break;
+ ret = ssz;
+ nerr("ERROR: send failed: %d\n", -ret);
+ goto errout_with_errno;
}
- sent += input_buffer_size;
+ ninfo("Sent %zd bytes request body at offset %ju "
+ "in the %zu bytes chunk, "
+ "total remaining %zu bytes\n",
+ ssz,
+ (uintmax_t)ws->state_offset,
+ ws->data_len,
+ ws->state_len);
+ ws->state_len -= ssz;
+ ws->state_offset += ssz;
+ DEBUGASSERT(ws->state_offset <= ws->data_len);
+ if (ws->state_offset == ws->data_len)
+ {
+ ws->data_buffer = NULL;
+ }
}
}
- if (ret < 0)
- {
- nerr("ERROR: send failed: %d\n", -ret);
- goto errout_with_errno;
- }
-
/* Now loop to get the file sent in response to the GET. This
* loop continues until either we read the end of file (nbytes == 0)
* or until we detect that we have been redirected.
*/
- ws->state = WEBCLIENT_STATE_STATUSLINE;
- redirected = false;
- for (; ; )
+ if (ws->state == WEBCLIENT_STATE_STATUSLINE ||
+ ws->state == WEBCLIENT_STATE_HEADERS ||
+ ws->state == WEBCLIENT_STATE_DATA)
{
- ws->datend = conn_recv(ctx, &conn, ws->buffer, ws->buflen);
- if (ws->datend < 0)
+ for (; ; )
{
- ret = ws->datend;
- nerr("ERROR: recv failed: %d\n", -ret);
- goto errout_with_errno;
- }
- else if (ws->datend == 0)
- {
- if (ws->state != WEBCLIENT_STATE_DATA)
+ ws->datend = conn_recv(ctx, conn, ws->buffer, ws->buflen);
+ if (ws->datend < 0)
{
- nerr("Connection lost unexpectedly\n");
- ret = -ECONNABORTED;
+ ret = ws->datend;
+ nerr("ERROR: recv failed: %d\n", -ret);
goto errout_with_errno;
}
+ else if (ws->datend == 0)
+ {
+ if (ws->state != WEBCLIENT_STATE_DATA)
+ {
+ nerr("Connection lost unexpectedly\n");
+ ret = -ECONNABORTED;
+ goto errout_with_errno;
+ }
- ninfo("Connection lost\n");
- break;
- }
+ ninfo("Connection lost\n");
+ ws->state = WEBCLIENT_STATE_CLOSE;
+ ws->redirected = 0;
+ break;
+ }
- /* Handle initial parsing of the status line */
+ /* Handle initial parsing of the status line */
- ws->offset = 0;
- if (ws->state == WEBCLIENT_STATE_STATUSLINE)
- {
- ret = wget_parsestatus(ctx, ws);
- if (ret < 0)
+ ws->offset = 0;
+ if (ws->state == WEBCLIENT_STATE_STATUSLINE)
{
- goto errout_with_errno;
+ ret = wget_parsestatus(ctx, ws);
+ if (ret < 0)
+ {
+ goto errout_with_errno;
+ }
}
- }
- /* Parse the HTTP data */
+ /* Parse the HTTP data */
- if (ws->state == WEBCLIENT_STATE_HEADERS)
- {
- ret = wget_parseheaders(ws);
- if (ret < 0)
+ if (ws->state == WEBCLIENT_STATE_HEADERS)
{
- goto errout_with_errno;
+ ret = wget_parseheaders(ws);
+ if (ret < 0)
+ {
+ goto errout_with_errno;
+ }
}
- }
- /* Dispose of the data payload */
+ /* Dispose of the data payload */
- if (ws->state == WEBCLIENT_STATE_DATA)
- {
- if (ws->httpstatus != HTTPSTATUS_MOVED)
+ if (ws->state == WEBCLIENT_STATE_DATA)
{
- /* Let the client decide what to do with the
- * received file.
- */
-
- if (ws->offset == ws->datend)
+ if (ws->httpstatus != HTTPSTATUS_MOVED)
{
- /* We don't have data to give to the client yet. */
- }
- else if (ctx->sink_callback)
- {
- ret = ctx->sink_callback(&ws->buffer, ws->offset,
- ws->datend, &ws->buflen,
- ctx->sink_callback_arg);
- if (ret != 0)
+ /* Let the client decide what to do with the
+ * received file.
+ */
+
+ if (ws->offset == ws->datend)
{
- goto errout_with_errno;
+ /* We don't have data to give to the client yet. */
+ }
+ else if (ctx->sink_callback)
+ {
+ ret = ctx->sink_callback(&ws->buffer, ws->offset,
+ ws->datend, &ws->buflen,
+ ctx->sink_callback_arg);
+ if (ret != 0)
+ {
+ goto errout_with_errno;
+ }
+ }
+ else
+ {
+ ctx->callback(&ws->buffer, ws->offset, ws->datend,
+ &ws->buflen, ctx->sink_callback_arg);
}
}
else
{
- ctx->callback(&ws->buffer, ws->offset, ws->datend,
- &ws->buflen, ctx->sink_callback_arg);
- }
- }
- else
- {
- redirected = true;
- nredirect++;
- if (nredirect > CONFIG_WEBCLIENT_MAX_REDIRECT)
- {
- nerr("ERROR: too many redirects (%u)\n", nredirect);
- goto errout;
- }
+ ws->nredirect++;
+ if (ws->nredirect > CONFIG_WEBCLIENT_MAX_REDIRECT)
+ {
+ nerr("ERROR: too many redirects (%u)\n",
+ ws->nredirect);
+ ret = -ELOOP;
+ goto errout_with_errno;
+ }
- break;
+ ws->state = WEBCLIENT_STATE_CLOSE;
+ ws->redirected = 1;
+ break;
+ }
}
}
}
- conn_close(ctx, &conn);
+ if (ws->state == WEBCLIENT_STATE_CLOSE)
+ {
+ conn_close(ctx, conn);
+ ws->need_conn_close = false;
+ if (ws->redirected)
+ {
+ ws->state = WEBCLIENT_STATE_SOCKET;
+ }
+ else
+ {
+ ws->state = WEBCLIENT_STATE_DONE;
+ }
+ }
}
- while (redirected);
+ while (ws->state != WEBCLIENT_STATE_DONE);
free(ws);
Review comment:
This is kinda pointless in situations like this. Dangling pointers are a thing if we are talking about global variables or library objects that are passed to library functions. But in this case? 1) return is called every time free() is called - thus ws is lost anyway 2) ws is accessed in this function directly via ws->, so even if you nullify this, you will still crash when calling ws->status or dereference ws in any way. Nullifying local variables before return is superflous.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org