| Message ID | 1418239610-3997-2-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 10 December 2014 at 19:26, Eduardo Habkost <ehabkost@redhat.com> wrote: > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > exec.c | 1 - > hw/virtio/virtio.c | 1 - > include/exec/exec-all.h | 2 ++ > 3 files changed, 2 insertions(+), 2 deletions(-) I thought this prototype was deliberately not in a generally included header file because it's really not something that should be needed by most code. If we do want to move it into a header then we definitely don't want it in exec-all.h. thanks -- PMM
On Wed, Dec 10, 2014 at 07:44:04PM +0000, Peter Maydell wrote: > On 10 December 2014 at 19:26, Eduardo Habkost <ehabkost@redhat.com> wrote: > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > exec.c | 1 - > > hw/virtio/virtio.c | 1 - > > include/exec/exec-all.h | 2 ++ > > 3 files changed, 2 insertions(+), 2 deletions(-) > > I thought this prototype was deliberately not in a generally > included header file because it's really not something that > should be needed by most code. If we do want to move it into > a header then we definitely don't want it in exec-all.h. I have no idea what would be the best place for the prototype, then. CCing the people who introduced the function, in case they have any suggestion. Anyway, I don't want to make this series depend on dealing with the virtio default-endianness mess, so in the next version I will drop this patch and simply put the prototype inside tests/x86-stub.c.
On Fri, 12 Dec 2014 16:20:16 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Wed, Dec 10, 2014 at 07:44:04PM +0000, Peter Maydell wrote: > > On 10 December 2014 at 19:26, Eduardo Habkost <ehabkost@redhat.com> wrote: > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > exec.c | 1 - > > > hw/virtio/virtio.c | 1 - > > > include/exec/exec-all.h | 2 ++ > > > 3 files changed, 2 insertions(+), 2 deletions(-) > > > > I thought this prototype was deliberately not in a generally > > included header file because it's really not something that > > should be needed by most code. If we do want to move it into > > a header then we definitely don't want it in exec-all.h. > > I have no idea what would be the best place for the prototype, then. Maybe include/qemu/bswap.h ? Thomas
On Fri, 12 Dec 2014 16:20:16 -0200 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Wed, Dec 10, 2014 at 07:44:04PM +0000, Peter Maydell wrote: > > On 10 December 2014 at 19:26, Eduardo Habkost <ehabkost@redhat.com> wrote: > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > > --- > > > exec.c | 1 - > > > hw/virtio/virtio.c | 1 - > > > include/exec/exec-all.h | 2 ++ > > > 3 files changed, 2 insertions(+), 2 deletions(-) > > > > I thought this prototype was not in a generally > > included header file because it's really not something that > > should be needed by most code. If we do want to move it into > > a header then we definitely don't want it in exec-all.h. > > I have no idea what would be the best place for the prototype, then. > CCing the people who introduced the function, in case they have any > suggestion. > > Anyway, I don't want to make this series depend on dealing with the > virtio default-endianness mess, so in the next version I will drop this > patch and simply put the prototype inside tests/x86-stub.c. > Indeed, this function is part of the legacy virtio endianness mess. And Peter's remark is true: the prototype was deliberately kept hidden since no code should need it except virtio. There are two users and this isn't likely to change (except when everyone will have switched to virtio-1 and we drop all the legacy virtio hacks): - virtio_default_endian() in hw/virtio/virtio.c - cpu_common_virtio_is_big_endian() in qom/cpu.c I don't think it is worth moving the prototype to a common header file. Perhaps a comment saying "this is a legacy virtio hack that should stay hidden" would be more appropriate. -- Greg
diff --git a/exec.c b/exec.c index 71ac104..6666ed5 100644 --- a/exec.c +++ b/exec.c @@ -2845,7 +2845,6 @@ int cpu_memory_rw_debug(CPUState *cpu, target_ulong addr, * A helper function for the _utterly broken_ virtio device model to find out if * it's running on a big endian machine. Don't do this at home kids! */ -bool target_words_bigendian(void); bool target_words_bigendian(void) { #if defined(TARGET_WORDS_BIGENDIAN) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 013979a..9df248d 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -552,7 +552,6 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val) vdev->status = val; } -bool target_words_bigendian(void); static enum virtio_device_endian virtio_default_endian(void) { if (target_words_bigendian()) { diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 0844885..d8ca313 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -383,4 +383,6 @@ static inline bool cpu_can_do_io(CPUState *cpu) return cpu->can_do_io != 0; } +bool target_words_bigendian(void); + #endif
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- exec.c | 1 - hw/virtio/virtio.c | 1 - include/exec/exec-all.h | 2 ++ 3 files changed, 2 insertions(+), 2 deletions(-)