Re: [kvm-unit-tests PATCH 19/18] don't embed code inside asserts

2015-11-10 Thread Paolo Bonzini 


On 09/11/2015 21:57, Andrew Jones wrote:
> assert() is classically a macro which could also be disabled, so if
> somebody introduces a switch to "#define assert(...) /*nothing*/" in
> the future, we'd lose code.
> 
> Suggested-by: Thomas Huth 
> Signed-off-by: Andrew Jones 
> ---
>  lib/arm/setup.c   | 19 ++-
>  lib/arm/smp.c |  4 +++-
>  lib/virtio-mmio.c |  4 +++-
>  3 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 02e81a689a8a6..da6edc1f9d8ff 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -39,8 +39,11 @@ static void cpu_set(int fdtnode __unused, u32 regval, void 
> *info __unused)
>  
>  static void cpu_init(void)
>  {
> + int ret;
> +
>   nr_cpus = 0;
> - assert(dt_for_each_cpu_node(cpu_set, NULL) == 0);
> + ret = dt_for_each_cpu_node(cpu_set, NULL);
> + assert(ret == 0);
>   set_cpu_online(0, true);
>  }
>  
> @@ -49,8 +52,10 @@ static void mem_init(phys_addr_t freemem_start)
>   /* we only expect one membank to be defined in the DT */
>   struct dt_pbus_reg regs[1];
>   phys_addr_t mem_start, mem_end;
> + int ret;
>  
> - assert(dt_get_memory_params(regs, 1));
> + ret = dt_get_memory_params(regs, 1);
> + assert(ret != 0);
>  
>   mem_start = regs[0].addr;
>   mem_end = mem_start + regs[0].size;
> @@ -71,14 +76,17 @@ void setup(const void *fdt)
>  {
>   const char *bootargs;
>   u32 fdt_size;
> + int ret;
>  
>   /*
>* Move the fdt to just above the stack. The free memory
>* then starts just after the fdt.
>*/
>   fdt_size = fdt_totalsize(fdt);
> - assert(fdt_move(fdt, , fdt_size) == 0);
> - assert(dt_init() == 0);
> + ret = fdt_move(fdt, , fdt_size);
> + assert(ret == 0);
> + ret = dt_init();
> + assert(ret == 0);
>  
>   mem_init(PAGE_ALIGN((unsigned long) + fdt_size));
>   io_init();
> @@ -86,6 +94,7 @@ void setup(const void *fdt)
>  
>   thread_info_init(current_thread_info(), 0);
>  
> - assert(dt_get_bootargs() == 0);
> + ret = dt_get_bootargs();
> + assert(ret == 0);
>   setup_args(bootargs);
>  }
> diff --git a/lib/arm/smp.c b/lib/arm/smp.c
> index 3cfc6d5ddedd0..390c53b5d84c3 100644
> --- a/lib/arm/smp.c
> +++ b/lib/arm/smp.c
> @@ -44,11 +44,13 @@ secondary_entry_fn secondary_cinit(void)
>  void smp_boot_secondary(int cpu, secondary_entry_fn entry)
>  {
>   void *stack_base = memalign(THREAD_SIZE, THREAD_SIZE);
> + int ret;
>  
>   secondary_data.stack = stack_base + THREAD_START_SP;
>   secondary_data.entry = entry;
>   mmu_mark_disabled(cpu);
> - assert(cpu_psci_cpu_boot(cpu) == 0);
> + ret = cpu_psci_cpu_boot(cpu);
> + assert(ret == 0);
>  
>   while (!cpu_online(cpu))
>   wfe();
> diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
> index 5ccbd193a264a..fa8dd5b8d484d 100644
> --- a/lib/virtio-mmio.c
> +++ b/lib/virtio-mmio.c
> @@ -123,10 +123,12 @@ static int vm_dt_match(const struct dt_device *dev, int 
> fdtnode)
>   struct vm_dt_info *info = (struct vm_dt_info *)dev->info;
>   struct dt_pbus_reg base;
>   u32 magic;
> + int ret;
>  
>   dt_device_bind_node((struct dt_device *)dev, fdtnode);
>  
> - assert(dt_pbus_get_base(dev, ) == 0);
> + ret = dt_pbus_get_base(dev, );
> + assert(ret == 0);
>   info->base = ioremap(base.addr, base.size);
>  
>   magic = readl(info->base + VIRTIO_MMIO_MAGIC_VALUE);
> 

Applied, thanks.

Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[kvm-unit-tests PATCH 19/18] don't embed code inside asserts

2015-11-09 Thread Andrew Jones 
assert() is classically a macro which could also be disabled, so if
somebody introduces a switch to "#define assert(...) /*nothing*/" in
the future, we'd lose code.

Suggested-by: Thomas Huth 
Signed-off-by: Andrew Jones 
---
 lib/arm/setup.c   | 19 ++-
 lib/arm/smp.c |  4 +++-
 lib/virtio-mmio.c |  4 +++-
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 02e81a689a8a6..da6edc1f9d8ff 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -39,8 +39,11 @@ static void cpu_set(int fdtnode __unused, u32 regval, void 
*info __unused)
 
 static void cpu_init(void)
 {
+   int ret;
+
nr_cpus = 0;
-   assert(dt_for_each_cpu_node(cpu_set, NULL) == 0);
+   ret = dt_for_each_cpu_node(cpu_set, NULL);
+   assert(ret == 0);
set_cpu_online(0, true);
 }
 
@@ -49,8 +52,10 @@ static void mem_init(phys_addr_t freemem_start)
/* we only expect one membank to be defined in the DT */
struct dt_pbus_reg regs[1];
phys_addr_t mem_start, mem_end;
+   int ret;
 
-   assert(dt_get_memory_params(regs, 1));
+   ret = dt_get_memory_params(regs, 1);
+   assert(ret != 0);
 
mem_start = regs[0].addr;
mem_end = mem_start + regs[0].size;
@@ -71,14 +76,17 @@ void setup(const void *fdt)
 {
const char *bootargs;
u32 fdt_size;
+   int ret;
 
/*
 * Move the fdt to just above the stack. The free memory
 * then starts just after the fdt.
 */
fdt_size = fdt_totalsize(fdt);
-   assert(fdt_move(fdt, , fdt_size) == 0);
-   assert(dt_init() == 0);
+   ret = fdt_move(fdt, , fdt_size);
+   assert(ret == 0);
+   ret = dt_init();
+   assert(ret == 0);
 
mem_init(PAGE_ALIGN((unsigned long) + fdt_size));
io_init();
@@ -86,6 +94,7 @@ void setup(const void *fdt)
 
thread_info_init(current_thread_info(), 0);
 
-   assert(dt_get_bootargs() == 0);
+   ret = dt_get_bootargs();
+   assert(ret == 0);
setup_args(bootargs);
 }
diff --git a/lib/arm/smp.c b/lib/arm/smp.c
index 3cfc6d5ddedd0..390c53b5d84c3 100644
--- a/lib/arm/smp.c
+++ b/lib/arm/smp.c
@@ -44,11 +44,13 @@ secondary_entry_fn secondary_cinit(void)
 void smp_boot_secondary(int cpu, secondary_entry_fn entry)
 {
void *stack_base = memalign(THREAD_SIZE, THREAD_SIZE);
+   int ret;
 
secondary_data.stack = stack_base + THREAD_START_SP;
secondary_data.entry = entry;
mmu_mark_disabled(cpu);
-   assert(cpu_psci_cpu_boot(cpu) == 0);
+   ret = cpu_psci_cpu_boot(cpu);
+   assert(ret == 0);
 
while (!cpu_online(cpu))
wfe();
diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c
index 5ccbd193a264a..fa8dd5b8d484d 100644
--- a/lib/virtio-mmio.c
+++ b/lib/virtio-mmio.c
@@ -123,10 +123,12 @@ static int vm_dt_match(const struct dt_device *dev, int 
fdtnode)
struct vm_dt_info *info = (struct vm_dt_info *)dev->info;
struct dt_pbus_reg base;
u32 magic;
+   int ret;
 
dt_device_bind_node((struct dt_device *)dev, fdtnode);
 
-   assert(dt_pbus_get_base(dev, ) == 0);
+   ret = dt_pbus_get_base(dev, );
+   assert(ret == 0);
info->base = ioremap(base.addr, base.size);
 
magic = readl(info->base + VIRTIO_MMIO_MAGIC_VALUE);
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html