[Spice-devel] [PATCH spice-gtk v4 12/29] fixup! usb-redir: add files for SCSI and USB MSC implementation
Frediano Ziglio
fziglio at redhat.com
Thu Aug 29 16:16:42 UTC 2019
> Frediano Ziglio writes:
>
> > Do not use G_GUINT32_FORMAT.
> > We support a minimum of 32 bit architectures.
>
> What was wrong with the old code?
>
Shorter and easier to read. I mean '%u' and '%" G_GUINT32_FORMAT "',
if you have an issue is harder to get it.
For instance it was hard to spot the misleading '0x%" G_GUINT32_FORMAT "'
which got '0x%u' so 100 would be formatted as 0x100 which I would read
as 256, not 100.
> If you want to get rid of G_ macro dependencies, why not use PRIu32?
>
No, G_ macros are fine, also taking into account that PRIu64 and G_GUINT64_FORMAT
could be different (for instance in Windows) and that the receiving formatting
function accepts better the G_ version (they are GLib).
> > ---
> > src/cd-usb-bulk-msd.c | 53 +++++++++++++++++++++----------------------
> > 1 file changed, 26 insertions(+), 27 deletions(-)
> >
> > diff --git a/src/cd-usb-bulk-msd.c b/src/cd-usb-bulk-msd.c
> > index 49e01eb6..5d95dac7 100644
> > --- a/src/cd-usb-bulk-msd.c
> > +++ b/src/cd-usb-bulk-msd.c
> > @@ -138,7 +138,7 @@ UsbCdBulkMsdDevice *cd_usb_bulk_msd_alloc(void
> > *usb_user_data, uint32_t max_luns
> > cd_usb_bulk_msd_set_state(cd, USB_CD_STATE_INIT);
> > cd->usb_user_data = usb_user_data;
> >
> > - SPICE_DEBUG("Alloc, max_luns:%" G_GUINT32_FORMAT, max_luns);
> > + SPICE_DEBUG("Alloc, max_luns:%u", max_luns);
> > return cd;
> > }
> >
> > @@ -155,7 +155,7 @@ int cd_usb_bulk_msd_realize(UsbCdBulkMsdDevice *cd,
> > uint32_t lun,
> >
> > rc = cd_scsi_dev_realize(cd->scsi_target, lun, &scsi_dev_params);
> > if (rc != 0) {
> > - SPICE_ERROR("Failed to realize lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_ERROR("Failed to realize lun:%u", lun);
> > return rc;
> > }
> >
> > @@ -165,7 +165,7 @@ int cd_usb_bulk_msd_realize(UsbCdBulkMsdDevice *cd,
> > uint32_t lun,
> > cd_scsi_dev_request_release(cd->scsi_target,
> > &cd->usb_req.scsi_req);
> > }
> >
> > - SPICE_DEBUG("Realize OK lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_DEBUG("Realize OK lun:%u", lun);
> > return 0;
> > }
> >
> > @@ -175,11 +175,11 @@ int cd_usb_bulk_msd_lock(UsbCdBulkMsdDevice *cd,
> > uint32_t lun, gboolean lock)
> >
> > rc = cd_scsi_dev_lock(cd->scsi_target, lun, lock);
> > if (rc != 0) {
> > - SPICE_ERROR("Failed to lock lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_ERROR("Failed to lock lun:%u", lun);
> > return rc;
> > }
> >
> > - SPICE_DEBUG("Lock OK lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_DEBUG("Lock OK lun:%u", lun);
> > return 0;
> > }
> >
> > @@ -190,11 +190,11 @@ int cd_usb_bulk_msd_load(UsbCdBulkMsdDevice *cd,
> > uint32_t lun,
> >
> > rc = cd_scsi_dev_load(cd->scsi_target, lun, media_params);
> > if (rc != 0) {
> > - SPICE_ERROR("Failed to load lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_ERROR("Failed to load lun:%u", lun);
> > return rc;
> > }
> >
> > - SPICE_DEBUG("Load OK lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_DEBUG("Load OK lun:%u", lun);
> > return 0;
> > }
> >
> > @@ -204,7 +204,7 @@ int cd_usb_bulk_msd_get_info(UsbCdBulkMsdDevice *cd,
> > uint32_t lun, CdScsiDeviceI
> >
> > rc = cd_scsi_dev_get_info(cd->scsi_target, lun, lun_info);
> > if (rc != 0) {
> > - SPICE_ERROR("Failed to get info lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_ERROR("Failed to get info lun:%u", lun);
> > return rc;
> > }
> >
> > @@ -217,11 +217,11 @@ int cd_usb_bulk_msd_unload(UsbCdBulkMsdDevice *cd,
> > uint32_t lun)
> >
> > rc = cd_scsi_dev_unload(cd->scsi_target, lun);
> > if (rc != 0) {
> > - SPICE_ERROR("Failed to unload lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_ERROR("Failed to unload lun:%u", lun);
> > return rc;
> > }
> >
> > - SPICE_DEBUG("Unload OK lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_DEBUG("Unload OK lun:%u", lun);
> > return 0;
> > }
> >
> > @@ -231,11 +231,11 @@ int cd_usb_bulk_msd_unrealize(UsbCdBulkMsdDevice *cd,
> > uint32_t lun)
> >
> > rc = cd_scsi_dev_unrealize(cd->scsi_target, lun);
> > if (rc != 0) {
> > - SPICE_ERROR("Unrealize lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_ERROR("Unrealize lun:%u", lun);
> > return rc;
> > }
> >
> > - SPICE_DEBUG("Unrealize lun:%" G_GUINT32_FORMAT, lun);
> > + SPICE_DEBUG("Unrealize lun:%u", lun);
> > return 0;
> > }
> >
> > @@ -264,7 +264,7 @@ static int parse_usb_msd_cmd(UsbCdBulkMsdDevice *cd,
> > uint8_t *buf, uint32_t cbw_
> > CdScsiRequest *scsi_req = &usb_req->scsi_req;
> >
> > if (cbw_len != sizeof(*cbw)) {
> > - SPICE_ERROR("CMD: Bad CBW size:%" G_GUINT32_FORMAT, cbw_len);
> > + SPICE_ERROR("CMD: Bad CBW size:%u", cbw_len);
> > return -1;
> > }
> > if (le32toh(cbw->sig) != 0x43425355) { /* MSD command signature */
> > @@ -304,10 +304,10 @@ static int parse_usb_msd_cmd(UsbCdBulkMsdDevice *cd,
> > uint8_t *buf, uint32_t cbw_
> >
> > scsi_req->lun = usb_req->lun;
> >
> > - SPICE_DEBUG("CMD lun:%" G_GUINT32_FORMAT " tag:%#x flags:%08x "
> > - "cdb_len:%" G_GUINT32_FORMAT " req_len:%" G_GUINT32_FORMAT,
> > - usb_req->lun, le32toh(cbw->tag), cbw->flags,
> > - scsi_req->cdb_len, usb_req->usb_req_len);
> > + SPICE_DEBUG("CMD lun:%u tag:%#x flags:%08x "
> > + "cdb_len:%u req_len:%u",
> > + usb_req->lun, le32toh(cbw->tag), cbw->flags,
> > + scsi_req->cdb_len, usb_req->usb_req_len);
> >
> > /* prepare status - CSW */
> > usb_req->csw.sig = htole32(0x53425355);
> > @@ -364,8 +364,8 @@ static void usb_cd_send_data_in(UsbCdBulkMsdDevice *cd,
> > uint32_t max_len)
> > uint32_t avail_len = usb_req->scsi_in_len - usb_req->xfer_len;
> > uint32_t send_len = MIN(avail_len, max_len);
> >
> > - SPICE_DEBUG("Data-in cmd tag 0x%x, remains %" G_GUINT32_FORMAT
> > - ", requested %" G_GUINT32_FORMAT ", send %"
> > G_GUINT32_FORMAT,
> > + SPICE_DEBUG("Data-in cmd tag 0x%x, remains %u"
> > + ", requested %u, send %u",
> > usb_req->csw.tag, avail_len, max_len, send_len);
> >
> > g_assert(max_len <= usb_req->usb_req_len);
> > @@ -396,7 +396,7 @@ int cd_usb_bulk_msd_read(UsbCdBulkMsdDevice *cd,
> > uint32_t max_len)
> > UsbCdBulkMsdRequest *usb_req = &cd->usb_req;
> > CdScsiRequest *scsi_req = &usb_req->scsi_req;
> >
> > - SPICE_DEBUG("msd_read, state: %s, len %" G_GUINT32_FORMAT,
> > + SPICE_DEBUG("msd_read, state: %s, len %u",
> > usb_cd_state_str(cd->state), max_len);
> >
> > switch (cd->state) {
> > @@ -408,8 +408,8 @@ int cd_usb_bulk_msd_read(UsbCdBulkMsdDevice *cd,
> > uint32_t max_len)
> > usb_cd_send_status(cd);
> > } else {
> > usb_req->bulk_in_len += max_len;
> > - SPICE_DEBUG("msd_read CSW, req incomplete, added len %"
> > G_GUINT32_FORMAT
> > - " saved len %" G_GUINT32_FORMAT,
> > + SPICE_DEBUG("msd_read CSW, req incomplete, added len %u"
> > + " saved len %u",
> > max_len, usb_req->bulk_in_len);
> > }
> > break;
> > @@ -419,8 +419,7 @@ int cd_usb_bulk_msd_read(UsbCdBulkMsdDevice *cd,
> > uint32_t max_len)
> > usb_cd_send_data_in(cd, max_len);
> > } else {
> > usb_req->bulk_in_len += max_len;
> > - SPICE_DEBUG("msd_read DATAIN, req incomplete, added len %"
> > G_GUINT32_FORMAT
> > - " saved len %" G_GUINT32_FORMAT,
> > + SPICE_DEBUG("msd_read DATAIN, req incomplete, added len %u
> > saved len %u",
> > max_len, usb_req->bulk_in_len);
> > }
> > break;
> > @@ -433,7 +432,7 @@ int cd_usb_bulk_msd_read(UsbCdBulkMsdDevice *cd,
> > uint32_t max_len)
> > break;
> >
> > default:
> > - SPICE_ERROR("Unexpected read state: %s, len %" G_GUINT32_FORMAT,
> > + SPICE_ERROR("Unexpected read state: %s, len %u",
> > usb_cd_state_str(cd->state), max_len);
> > goto fail;
> > }
> > @@ -507,7 +506,7 @@ int cd_usb_bulk_msd_write(UsbCdBulkMsdDevice *cd,
> > uint8_t *buf_out, uint32_t buf
> > cd_usb_bulk_msd_set_state(cd, USB_CD_STATE_CSW); /* Status next */
> > break;
> > default:
> > - SPICE_DEBUG("Unexpected write state: %s, len %" G_GUINT32_FORMAT,
> > + SPICE_DEBUG("Unexpected write state: %s, len %u",
> > usb_cd_state_str(cd->state), buf_out_len);
> > goto fail;
> > }
> > @@ -536,7 +535,7 @@ void cd_scsi_target_reset_complete(void
> > *target_user_data)
> > void cd_scsi_dev_changed(void *target_user_data, uint32_t lun)
> > {
> > UsbCdBulkMsdDevice *cd = (UsbCdBulkMsdDevice *)target_user_data;
> > - SPICE_DEBUG("Device changed, state: %s lun: %" G_GUINT32_FORMAT,
> > + SPICE_DEBUG("Device changed, state: %s lun: %u",
> > usb_cd_state_str(cd->state), lun);
> > cd_usb_bulk_msd_lun_changed(cd->usb_user_data, lun);
> > }
More information about the Spice-devel
mailing list