Re: [PATCH spice-gtk v4 12/29] fixup! usb-redir: add files for SCSI and USB MSC implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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);
> >  }
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]