pkarashchenko commented on code in PR #1666:
URL: https://github.com/apache/nuttx-apps/pull/1666#discussion_r1141760240
##########
system/ymodem/rb_main.c:
##########
@@ -106,20 +107,23 @@ static int handler(FAR struct ymodem_ctx *ctx)
size = ctx->packet_size;
}
- ret = write(ym_fd->file_fd, ctx->data, size);
- if (ret < 0)
+ i = 0;
+ while (i < size)
{
- return -errno;
- }
- else if (ret < size)
- {
- return ERROR;
+ ret = write(ym_fd->file_fd, ctx->data + i, size - i);
+ if (ret < 0)
+ {
+ return ret;
+ }
+
+ i += ret;
Review Comment:
```suggestion
}
else
{
i += ret;
}
```
##########
system/ymodem/rb_main.c:
##########
@@ -176,40 +183,74 @@ int main(int argc, FAR char *argv[])
strlcpy(ym_fd.pathname, optarg, PATH_MAX);
if (ym_fd.pathname[strlen(ym_fd.pathname)] == '/')
{
- ym_fd.pathname[strlen(ym_fd.pathname)] = 0;
+ ym_fd.pathname[strlen(ym_fd.pathname)] = '\0';
}
break;
case 'd':
- ctx.recvfd = open(optarg, O_RDWR | O_NONBLOCK);
- if (ctx.recvfd < 0)
+ recvfd = open(optarg, O_RDWR);
+ if (recvfd < 0)
{
fprintf(stderr, "ERROR: can't open %s\n", optarg);
+ return recvfd;
}
- ctx.sendfd = ctx.recvfd;
break;
- case 'h':
- show_usage(argv[0], EXIT_FAILURE);
+ case 'k':
+ customsize = atoi(optarg) * 1024;
break;
- default:
+ case 't':
+ timeout = atoi(optarg);
+ if (timeout != 0)
+ {
+ break;
+ }
+
+ case 'h':
case '?':
- fprintf(stderr, "ERROR: Unrecognized option\n");
+ default:
show_usage(argv[0], EXIT_FAILURE);
break;
}
}
- ymodem_recv(&ctx);
- if (ctx.recvfd)
+ ctx = malloc(sizeof(*ctx));
+ if (ctx == NULL)
{
- close(ctx.recvfd);
+ fprintf(stderr, "ERROR: can't malloc ymodem ctx\n");
+ ret = -ENOMEM;
+ goto out;
}
+ memset(ctx, 0, sizeof(struct ymodem_ctx));
Review Comment:
can `calloc` and remove `memset`
##########
system/ymodem/sb_main.c:
##########
@@ -108,55 +131,84 @@ static void show_usage(FAR const char *progname, int
errcode)
int main(int argc, FAR char *argv[])
{
struct ymodem_fd ym_fd;
- struct ymodem_ctx ctx;
+ struct ymodem_ctx *ctx;
Review Comment:
```suggestion
FAR struct ymodem_ctx *ctx;
```
##########
system/ymodem/rb_main.c:
##########
@@ -176,40 +183,74 @@ int main(int argc, FAR char *argv[])
strlcpy(ym_fd.pathname, optarg, PATH_MAX);
if (ym_fd.pathname[strlen(ym_fd.pathname)] == '/')
{
- ym_fd.pathname[strlen(ym_fd.pathname)] = 0;
+ ym_fd.pathname[strlen(ym_fd.pathname)] = '\0';
}
break;
case 'd':
- ctx.recvfd = open(optarg, O_RDWR | O_NONBLOCK);
- if (ctx.recvfd < 0)
+ recvfd = open(optarg, O_RDWR);
+ if (recvfd < 0)
{
fprintf(stderr, "ERROR: can't open %s\n", optarg);
+ return recvfd;
}
- ctx.sendfd = ctx.recvfd;
break;
- case 'h':
- show_usage(argv[0], EXIT_FAILURE);
+ case 'k':
+ customsize = atoi(optarg) * 1024;
break;
- default:
+ case 't':
+ timeout = atoi(optarg);
+ if (timeout != 0)
+ {
+ break;
+ }
+
+ case 'h':
case '?':
- fprintf(stderr, "ERROR: Unrecognized option\n");
+ default:
show_usage(argv[0], EXIT_FAILURE);
break;
}
}
- ymodem_recv(&ctx);
- if (ctx.recvfd)
+ ctx = malloc(sizeof(*ctx));
+ if (ctx == NULL)
{
- close(ctx.recvfd);
+ fprintf(stderr, "ERROR: can't malloc ymodem ctx\n");
+ ret = -ENOMEM;
+ goto out;
}
+ memset(ctx, 0, sizeof(struct ymodem_ctx));
+ ctx->packet_handler = handler;
+ ctx->customsize = customsize;
+ ctx->timeout = timeout;
+ ctx->priv = &ym_fd;
+ if (recvfd)
+ {
+ ctx->recvfd = recvfd;
+ ctx->sendfd = recvfd;
Review Comment:
So both `recvfd`and `sendfd` are set to the same value? What is the point
with that?
##########
system/ymodem/ymodem.h:
##########
@@ -55,7 +55,8 @@ struct ymodem_ctx
{
uint8_t header;
uint8_t seq[2];
- uint8_t data[YMODEM_PACKET_1K_SIZE];
+ uint8_t *data;
Review Comment:
```suggestion
FAR uint8_t *data;
```
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
