Hi Isaac,
Some minor comments below.
Isaac Lozano <109loza...@gmail.com> writes:
> MTP requires that if a file is larger than 4gb or if sending data larger
> than 4gb, that the length field be set to 0xFFFFFFFF.
>
> Also widened a couple variables to prevent overflow errors.
>
> Signed-off-by: Isaac Lozano <109loza...@gmail.com>
> ---
> hw/usb/dev-mtp.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
> index 1be85ae..6f6a270 100644
> --- a/hw/usb/dev-mtp.c
> +++ b/hw/usb/dev-mtp.c
> @@ -115,8 +115,8 @@ struct MTPControl {
> struct MTPData {
> uint16_t code;
> uint32_t trans;
> - uint32_t offset;
> - uint32_t length;
> + uint64_t offset;
> + uint64_t length;
> uint32_t alloc;
> uint8_t *data;
> bool first;
> @@ -883,7 +883,13 @@ static MTPData *usb_mtp_get_object_info(MTPState *s,
> MTPControl *c,
> usb_mtp_add_u32(d, QEMU_STORAGE_ID);
> usb_mtp_add_u16(d, o->format);
> usb_mtp_add_u16(d, 0);
> - usb_mtp_add_u32(d, o->stat.st_size);
> +
> + if (o->stat.st_size > 0xFFFFFFFF) {
> + usb_mtp_add_u32(d, 0xFFFFFFFF);
> + }
> + else {
> + usb_mtp_add_u32(d, o->stat.st_size);
> + }
Or rewrite as:
uint32_t size =
o->stat.st_size > 0xFFFFFFFF ? 0xFFFFFFFF : o->stat.st_size;
usb_mtp_add_u32(d, size);
Either way is fine.
>
> usb_mtp_add_u16(d, 0);
> usb_mtp_add_u32(d, 0);
> @@ -1193,10 +1199,15 @@ static void usb_mtp_handle_data(USBDevice *dev,
> USBPacket *p)
> }
> if (s->data_in != NULL) {
> MTPData *d = s->data_in;
> - int dlen = d->length - d->offset;
> + uint64_t dlen = d->length - d->offset;
> if (d->first) {
> trace_usb_mtp_data_in(s->dev.addr, d->trans, d->length);
> - container.length = cpu_to_le32(d->length +
> sizeof(container));
> + if (d->length + sizeof(container) > 0xFFFFFFFF) {
> + container.length = cpu_to_le32(0xFFFFFFFF);
> + }
> + else {
> + container.length = cpu_to_le32(d->length +
> sizeof(container));
> + }
This will throw checkpatch errors and I see you have fixed them in the next
patch.
Please just use the preferred style here to keep context.
> container.type = cpu_to_le16(TYPE_DATA);
> container.code = cpu_to_le16(d->code);
> container.trans = cpu_to_le32(d->trans);
Please feel free to add
Reviewed-by: Bandan Das <b...@redhat.com>
