Re: [PATCH 6/7] hw/isa: Assert isa_register_portio_list() gets non-NULL ISA device
On 14/2/23 19:49, Richard Henderson wrote:
On 2/14/23 00:18, Philippe Mathieu-Daudé wrote:
__attribute__((nonnull)) void a1(void *ptr)
{
// can no use assert(ptr) because compiler warning
}
I briefly glossed over that...
I realize we'd probably want to add -fno-delete-null-pointer-checks
if we make too much
... here. The compiler warning should go away with the right flag.
Doh, got it now!
Re: [PATCH 6/7] hw/isa: Assert isa_register_portio_list() gets non-NULL ISA device
On 2/14/23 00:18, Philippe Mathieu-Daudé wrote:
__attribute__((nonnull)) void a1(void *ptr)
{
// can no use assert(ptr) because compiler warning
}
I briefly glossed over that...
I realize we'd probably want to add -fno-delete-null-pointer-checks if we make too much
... here. The compiler warning should go away with the right flag.
r~
Re: [PATCH 6/7] hw/isa: Assert isa_register_portio_list() gets non-NULL ISA device
On 8/2/23 20:47, Richard Henderson wrote:
On 2/7/23 14:07, Philippe Mathieu-Daudé wrote:
The previous commit removed the single call to
isa_register_portio_list() with dev=NULL. To be
sure we won't reintroduce such weird (ab)use,
add an assertion.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
I wonder how much use of __attribute__((nonnull)) we should be making.
__attribute__((nonnull)) is compile-time, but seems weaker than the
good old runtime assert():
void a0(void *ptr)
{
assert(ptr);
}
__attribute__((nonnull)) void a1(void *ptr)
{
// can no use assert(ptr) because compiler warning
}
void b0(void *x)
{
a(NULL); // runtime assertion
}
void b(void *x)
{
a1(NULL); // compile error
}
void c0(void *x)
{
a0(x);
}
void c1(void *x)
{
a1(x);
}
void d0(void *x)
{
c0(NULL); // runtime assertion
}
void d1(void *x)
{
c1(NULL); // no compile error, no assertion!
}
I realize we'd probably want to add -fno-delete-null-pointer-checks if
we make too much use of that.
Re: [PATCH 6/7] hw/isa: Assert isa_register_portio_list() gets non-NULL ISA device
On 2/7/23 14:07, Philippe Mathieu-Daudé wrote: The previous commit removed the single call to isa_register_portio_list() with dev=NULL. To be sure we won't reintroduce such weird (ab)use, add an assertion. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson I wonder how much use of __attribute__((nonnull)) we should be making. I realize we'd probably want to add -fno-delete-null-pointer-checks if we make too much use of that. r~
[PATCH 6/7] hw/isa: Assert isa_register_portio_list() gets non-NULL ISA device
The previous commit removed the single call to
isa_register_portio_list() with dev=NULL. To be
sure we won't reintroduce such weird (ab)use,
add an assertion.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/isa/isa-bus.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 95fc1ba5f7..3d1996c115 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -107,7 +107,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan)
static inline void isa_init_ioport(ISADevice *dev, uint16_t ioport)
{
-if (dev && (dev->ioport_id == 0 || ioport < dev->ioport_id)) {
+if (dev->ioport_id == 0 || ioport < dev->ioport_id) {
dev->ioport_id = ioport;
}
}
@@ -123,6 +123,8 @@ int isa_register_portio_list(ISADevice *dev,
const MemoryRegionPortio *pio_start,
void *opaque, const char *name)
{
+assert(dev);
+
if (!isabus) {
return -ENODEV;
}
--
2.38.1
