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>