Message ID | 20160310093408.7a76ed94@t450s.home (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 03/10/2016 09:34 AM, Alex Williamson wrote: >>> + trace_vfio_msix_fixup(vdev->vbasedev.name, >>> + vdev->msix->table_bar, region->mmaps[0].offset, >>> + region->mmaps[0].offset + region->mmaps[0].size); >> Sorry this does not compile for me on arm 32b: >> >> ./trace/generated-tracers.h:16113:23: error: format ‘%lx’ expects >> argument of type ‘long unsigned int’, but argument 8 has type ‘off_t’ >> [-Werror=format=] , name, bar, offset, size); >> >> -> vfio_msix_fixup(const char *name, int bar, off_t start, off_t end) " >> (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]" ? > > vfio_msix_disable(const char *name) " (%s)" > -vfio_msix_fixup(const char *name, int bar, off_t offset, size_t size) " (%s) MSI-X region %d mmap fixup [0x%lx - 0x%lx]" > +vfio_msix_fixup(const char *name, int bar, off_t start, off_t end) " (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]" off_t and PRIx64 are not necessarily compatible types (on a 64-bit platform, one could be 'long' while the other is 'long long'). And even though we set compiler flags to get 64-bit off_t on 32-bit platforms, your code is not portable to people that don't set those flags and are stuck with 32-bit off_t. It may be better to declare start and end as [u]int64_t, rather than off_t.
On Thu, 10 Mar 2016 13:46:06 -0700 Eric Blake <eblake@redhat.com> wrote: > On 03/10/2016 09:34 AM, Alex Williamson wrote: > > >>> + trace_vfio_msix_fixup(vdev->vbasedev.name, > >>> + vdev->msix->table_bar, region->mmaps[0].offset, > >>> + region->mmaps[0].offset + region->mmaps[0].size); > >> Sorry this does not compile for me on arm 32b: > >> > >> ./trace/generated-tracers.h:16113:23: error: format ‘%lx’ expects > >> argument of type ‘long unsigned int’, but argument 8 has type ‘off_t’ > >> [-Werror=format=] , name, bar, offset, size); > >> > >> -> vfio_msix_fixup(const char *name, int bar, off_t start, off_t end) " > >> (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]" ? > > > > > vfio_msix_disable(const char *name) " (%s)" > > -vfio_msix_fixup(const char *name, int bar, off_t offset, size_t size) " (%s) MSI-X region %d mmap fixup [0x%lx - 0x%lx]" > > +vfio_msix_fixup(const char *name, int bar, off_t start, off_t end) " (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]" > > off_t and PRIx64 are not necessarily compatible types (on a 64-bit > platform, one could be 'long' while the other is 'long long'). And even > though we set compiler flags to get 64-bit off_t on 32-bit platforms, > your code is not portable to people that don't set those flags and are > stuck with 32-bit off_t. > > It may be better to declare start and end as [u]int64_t, rather than off_t. Looks like we need another respin anyway, and uint64_t works just as well here. Done. Thanks, Alex
diff --git a/trace-events b/trace-events index e36bc07..fd07512 100644 --- a/trace-events +++ b/trace-events @@ -1652,7 +1652,7 @@ vfio_msix_enable(const char *name) " (%s)" vfio_msix_pba_disable(const char *name) " (%s)" vfio_msix_pba_enable(const char *name) " (%s)" vfio_msix_disable(const char *name) " (%s)" -vfio_msix_fixup(const char *name, int bar, off_t offset, size_t size) " (%s) MSI-X region %d mmap fixup [0x%lx - 0x%lx]" +vfio_msix_fixup(const char *name, int bar, off_t start, off_t end) " (%s) MSI-X region %d mmap fixup [0x%"PRIx64" - 0x%"PRIx64"]" vfio_msi_enable(const char *name, int nr_vectors) " (%s) Enabled %d MSI vectors" vfio_msi_disable(const char *name) " (%s)" vfio_pci_load_rom(const char *name, unsigned long size, unsigned long offset, unsigned long flags) "Device %s ROM:\n size: 0x%lx, offset: 0x%lx, flags: 0x%lx"