Arjan van de Ven
2005-Oct-27 10:29 UTC
[Xen-devel] [patch] Lets not put statements with side-effects inside BUG_ON()
Hi, BUG_ON() is a macro very very similar to assert(), and it''s a really bad idea in general to put statements with side-effects inside such a construct (for example the BUG_ON() could be compiled away for non-debug builds). The patch below fixes this for the drivers/xen tree. I chose the general construct to store the respective return values in a local var instead of an if (...) BUG(); because it looks like most of these BUG_ON()''s need to be replaced by real error handling when going towards production releases anyway, and then the local variable is going to be needed anyway. (and besides the if construct tends to get unreable due to the unlikely() etc). Please apply. Greetings, Arjan van de Ven diff -purN linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c --- linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c 2005-10-26 17:13:26.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c 2005-10-27 11:26:28.000000000 +0200 @@ -191,12 +191,13 @@ static int increase_reservation(unsigned rc = HYPERVISOR_memory_op( XENMEM_increase_reservation, &reservation); if (rc < nr_pages) { + int ret; /* We hit the Xen hard limit: reprobe. */ reservation.extent_start = mfn_list; reservation.nr_extents = rc; - BUG_ON(HYPERVISOR_memory_op( - XENMEM_decrease_reservation, - &reservation) != rc); + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, + &reservation); + BUG_ON(ret != rc); hard_limit = current_pages + rc - driver_pages; goto out; } @@ -213,11 +214,14 @@ static int increase_reservation(unsigned xen_machphys_update(mfn_list[i], pfn); /* Link back into the page tables if not highmem. */ - if (pfn < max_low_pfn) - BUG_ON(HYPERVISOR_update_va_mapping( + if (pfn < max_low_pfn) { + int ret; + ret = HYPERVISOR_update_va_mapping( (unsigned long)__va(pfn << PAGE_SHIFT), pfn_pte_ma(mfn_list[i], PAGE_KERNEL), - 0)); + 0); + BUG_ON(ret); + } /* Relinquish the page back to the allocator. */ ClearPageReserved(page); @@ -242,6 +246,7 @@ static int decrease_reservation(unsigned struct page *page; void *v; int need_sleep = 0; + int ret; struct xen_memory_reservation reservation = { .address_bits = 0, .extent_order = 0, @@ -268,8 +273,9 @@ static int decrease_reservation(unsigned if (!PageHighMem(page)) { v = phys_to_virt(pfn << PAGE_SHIFT); scrub_pages(v, 1); - BUG_ON(HYPERVISOR_update_va_mapping( - (unsigned long)v, __pte_ma(0), 0)); + ret = HYPERVISOR_update_va_mapping( + (unsigned long)v, __pte_ma(0), 0); + BUG_ON(ret); } #ifdef CONFIG_XEN_SCRUB_PAGES else { @@ -295,8 +301,8 @@ static int decrease_reservation(unsigned reservation.extent_start = mfn_list; reservation.nr_extents = nr_pages; - BUG_ON(HYPERVISOR_memory_op( - XENMEM_decrease_reservation, &reservation) != nr_pages); + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation); + BUG_ON(ret != nr_pages); current_pages -= nr_pages; totalram_pages = current_pages; @@ -501,6 +507,7 @@ static int dealloc_pte_fn( pte_t *pte, struct page *pte_page, unsigned long addr, void *data) { unsigned long mfn = pte_mfn(*pte); + int ret; struct xen_memory_reservation reservation = { .extent_start = &mfn, .nr_extents = 1, @@ -510,8 +517,8 @@ static int dealloc_pte_fn( set_pte_at(&init_mm, addr, pte, __pte_ma(0)); phys_to_machine_mapping[__pa(addr) >> PAGE_SHIFT] INVALID_P2M_ENTRY; - BUG_ON(HYPERVISOR_memory_op( - XENMEM_decrease_reservation, &reservation) != 1); + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation); + BUG_ON(ret != 1); return 0; } @@ -519,6 +526,7 @@ struct page *balloon_alloc_empty_page_ra { unsigned long vstart, flags; unsigned int order = get_order(nr_pages * PAGE_SIZE); + int ret; vstart = __get_free_pages(GFP_KERNEL, order); if (vstart == 0) @@ -527,8 +535,9 @@ struct page *balloon_alloc_empty_page_ra scrub_pages(vstart, 1 << order); balloon_lock(flags); - BUG_ON(generic_page_range( - &init_mm, vstart, PAGE_SIZE << order, dealloc_pte_fn, NULL)); + ret = generic_page_range( + &init_mm, vstart, PAGE_SIZE << order, dealloc_pte_fn, NULL); + BUG_ON(ret); current_pages -= 1UL << order; balloon_unlock(flags); diff -purN linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c --- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c 2005-10-22 10:12:23.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c 2005-10-27 11:16:54.000000000 +0200 @@ -108,6 +108,7 @@ static void fast_flush_area(int idx, int struct gnttab_unmap_grant_ref unmap[BLKIF_MAX_SEGMENTS_PER_REQUEST]; unsigned int i, invcount = 0; u16 handle; + int ret; for (i = 0; i < nr_pages; i++) { handle = pending_handle(idx, i); @@ -120,8 +121,9 @@ static void fast_flush_area(int idx, int invcount++; } - BUG_ON(HYPERVISOR_grant_table_op( - GNTTABOP_unmap_grant_ref, unmap, invcount)); + ret = HYPERVISOR_grant_table_op( + GNTTABOP_unmap_grant_ref, unmap, invcount); + BUG_ON(ret); } @@ -338,6 +340,7 @@ static void dispatch_rw_block_io(blkif_t struct bio *bio = NULL, *biolist[BLKIF_MAX_SEGMENTS_PER_REQUEST]; int nbio = 0; request_queue_t *q; + int ret; /* Check that number of segments is sane. */ nseg = req->nr_segments; @@ -367,8 +370,8 @@ static void dispatch_rw_block_io(blkif_t map[i].flags |= GNTMAP_readonly; } - BUG_ON(HYPERVISOR_grant_table_op( - GNTTABOP_map_grant_ref, map, nseg)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg); + BUG_ON(ret); for (i = 0; i < nseg; i++) { if (unlikely(map[i].handle < 0)) { @@ -493,6 +496,7 @@ static int __init blkif_init(void) { int i; struct page *page; + int ret; blkif_interface_init(); @@ -509,7 +513,8 @@ static int __init blkif_init(void) spin_lock_init(&blkio_schedule_list_lock); INIT_LIST_HEAD(&blkio_schedule_list); - BUG_ON(kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES) < 0); + ret = kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES); + BUG_ON(ret < 0); blkif_xenbus_init(); diff -purN linux-2.6-xen-sparse/drivers/xen/blkback/interface.c linux-2.6-xen-sparse/drivers/xen/blkback/interface.c --- linux-2.6-xen-sparse/drivers/xen/blkback/interface.c 2005-10-22 10:12:24.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/blkback/interface.c 2005-10-27 11:17:29.000000000 +0200 @@ -31,6 +31,7 @@ blkif_t *alloc_blkif(domid_t domid) static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) { struct gnttab_map_grant_ref op; + int ret; op.host_addr = (unsigned long)blkif->blk_ring_area->addr; op.flags = GNTMAP_host_map; @@ -38,8 +39,9 @@ static int map_frontend_page(blkif_t *bl op.dom = blkif->domid; lock_vm_area(blkif->blk_ring_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); unlock_vm_area(blkif->blk_ring_area); + BUG_ON(ret); if (op.handle < 0) { DPRINTK(" Grant table operation failure !\n"); @@ -55,14 +57,16 @@ static int map_frontend_page(blkif_t *bl static void unmap_frontend_page(blkif_t *blkif) { struct gnttab_unmap_grant_ref op; + int ret; op.host_addr = (unsigned long)blkif->blk_ring_area->addr; op.handle = blkif->shmem_handle; op.dev_bus_addr = 0; lock_vm_area(blkif->blk_ring_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); unlock_vm_area(blkif->blk_ring_area); + BUG_ON(ret); } int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) diff -purN linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c --- linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c 2005-10-22 10:12:27.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c 2005-10-27 11:15:41.000000000 +0200 @@ -305,6 +305,7 @@ static irqreturn_t blkif_int(int irq, vo for (i = info->ring.rsp_cons; i != rp; i++) { unsigned long id; + int ret; bret = RING_GET_RESPONSE(&info->ring, i); id = bret->id; @@ -321,9 +322,10 @@ static irqreturn_t blkif_int(int irq, vo DPRINTK("Bad return from blkdev data " "request: %x\n", bret->status); - BUG_ON(end_that_request_first( + ret = end_that_request_first( req, (bret->status == BLKIF_RSP_OKAY), - req->hard_nr_sectors)); + req->hard_nr_sectors); + BUG_ON(ret); end_that_request_last(req); break; default: diff -purN linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c --- linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c 2005-10-22 10:12:30.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c 2005-10-27 11:19:42.000000000 +0200 @@ -413,6 +413,7 @@ static void fast_flush_area(int idx, int unsigned int i, op = 0; struct grant_handle_pair *handle; unsigned long ptep; + int ret; for ( i = 0; i < nr_pages; i++) { @@ -440,8 +441,8 @@ static void fast_flush_area(int idx, int BLKTAP_INVALIDATE_HANDLE(handle); } - BUG_ON(HYPERVISOR_grant_table_op( - GNTTABOP_unmap_grant_ref, unmap, op)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, op); + BUG_ON(ret); if (blktap_vma != NULL) zap_page_range(blktap_vma, @@ -673,6 +674,7 @@ static void dispatch_rw_block_io(blkif_t struct gnttab_map_grant_ref map[BLKIF_MAX_SEGMENTS_PER_REQUEST*2]; int op, ret; unsigned int nseg; + int retval; /* Check that number of segments is sane. */ nseg = req->nr_segments; @@ -740,8 +742,8 @@ static void dispatch_rw_block_io(blkif_t op++; } - BUG_ON(HYPERVISOR_grant_table_op( - GNTTABOP_map_grant_ref, map, op)); + retval = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, op); + BUG_ON(retval); op = 0; for (i = 0; i < (req->nr_segments*2); i += 2) { @@ -877,7 +879,8 @@ static int __init blkif_init(void) spin_lock_init(&blkio_schedule_list_lock); INIT_LIST_HEAD(&blkio_schedule_list); - BUG_ON(kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES) < 0); + i = kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES); + BUG_ON(i<0); blkif_xenbus_init(); diff -purN linux-2.6-xen-sparse/drivers/xen/blktap/interface.c linux-2.6-xen-sparse/drivers/xen/blktap/interface.c --- linux-2.6-xen-sparse/drivers/xen/blktap/interface.c 2005-10-22 10:12:31.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/blktap/interface.c 2005-10-27 11:20:19.000000000 +0200 @@ -31,6 +31,7 @@ blkif_t *alloc_blkif(domid_t domid) static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) { struct gnttab_map_grant_ref op; + int ret; op.host_addr = (unsigned long)blkif->blk_ring_area->addr; op.flags = GNTMAP_host_map; @@ -38,8 +39,9 @@ static int map_frontend_page(blkif_t *bl op.dom = blkif->domid; lock_vm_area(blkif->blk_ring_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); unlock_vm_area(blkif->blk_ring_area); + BUG_ON(ret); if (op.handle < 0) { DPRINTK(" Grant table operation failure !\n"); @@ -55,14 +57,16 @@ static int map_frontend_page(blkif_t *bl static void unmap_frontend_page(blkif_t *blkif) { struct gnttab_unmap_grant_ref op; + int ret; op.host_addr = (unsigned long)blkif->blk_ring_area->addr; op.handle = blkif->shmem_handle; op.dev_bus_addr = 0; lock_vm_area(blkif->blk_ring_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); unlock_vm_area(blkif->blk_ring_area); + BUG_ON(ret); } int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int evtchn) diff -purN linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c --- linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c 2005-10-22 10:12:37.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c 2005-10-27 11:27:29.000000000 +0200 @@ -282,6 +282,7 @@ static int evtchn_ioctl(struct inode *in case IOCTL_EVTCHN_UNBIND: { struct ioctl_evtchn_unbind unbind; + int ret; rc = -EFAULT; if (copy_from_user(&unbind, (void *)arg, sizeof(unbind))) @@ -306,7 +307,8 @@ static int evtchn_ioctl(struct inode *in op.cmd = EVTCHNOP_close; op.u.close.port = unbind.port; - BUG_ON(HYPERVISOR_event_channel_op(&op)); + ret = HYPERVISOR_event_channel_op(&op); + BUG_ON(ret); rc = 0; break; @@ -399,6 +401,7 @@ static int evtchn_release(struct inode * for (i = 0; i < NR_EVENT_CHANNELS; i++) { + int ret; if (port_user[i] != u) continue; @@ -407,7 +410,8 @@ static int evtchn_release(struct inode * op.cmd = EVTCHNOP_close; op.u.close.port = i; - BUG_ON(HYPERVISOR_event_channel_op(&op)); + ret = HYPERVISOR_event_channel_op(&op); + BUG_ON(ret); } spin_unlock_irq(&port_user_lock); diff -purN linux-2.6-xen-sparse/drivers/xen/netback/interface.c linux-2.6-xen-sparse/drivers/xen/netback/interface.c --- linux-2.6-xen-sparse/drivers/xen/netback/interface.c 2005-10-22 10:12:38.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/netback/interface.c 2005-10-27 11:10:20.000000000 +0200 @@ -115,6 +115,7 @@ static int map_frontend_pages( netif_t *netif, grant_ref_t tx_ring_ref, grant_ref_t rx_ring_ref) { struct gnttab_map_grant_ref op; + int ret; op.host_addr = (unsigned long)netif->comms_area->addr; op.flags = GNTMAP_host_map; @@ -122,8 +123,9 @@ static int map_frontend_pages( op.dom = netif->domid; lock_vm_area(netif->comms_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); unlock_vm_area(netif->comms_area); + BUG_ON(ret); if (op.handle < 0) { DPRINTK(" Gnttab failure mapping tx_ring_ref!\n"); @@ -139,8 +141,9 @@ static int map_frontend_pages( op.dom = netif->domid; lock_vm_area(netif->comms_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); unlock_vm_area(netif->comms_area); + BUG_ON(ret); if (op.handle < 0) { DPRINTK(" Gnttab failure mapping rx_ring_ref!\n"); @@ -156,22 +159,25 @@ static int map_frontend_pages( static void unmap_frontend_pages(netif_t *netif) { struct gnttab_unmap_grant_ref op; + int ret; op.host_addr = (unsigned long)netif->comms_area->addr; op.handle = netif->tx_shmem_handle; op.dev_bus_addr = 0; lock_vm_area(netif->comms_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); unlock_vm_area(netif->comms_area); + BUG_ON(ret); op.host_addr = (unsigned long)netif->comms_area->addr + PAGE_SIZE; op.handle = netif->rx_shmem_handle; op.dev_bus_addr = 0; lock_vm_area(netif->comms_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); unlock_vm_area(netif->comms_area); + BUG_ON(ret); } int netif_map(netif_t *netif, unsigned long tx_ring_ref, diff -purN linux-2.6-xen-sparse/drivers/xen/netback/netback.c linux-2.6-xen-sparse/drivers/xen/netback/netback.c --- linux-2.6-xen-sparse/drivers/xen/netback/netback.c 2005-10-22 10:12:39.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/netback/netback.c 2005-10-27 11:12:42.000000000 +0200 @@ -112,9 +112,12 @@ static void free_mfn(unsigned long mfn) spin_lock_irqsave(&mfn_lock, flags); if ( alloc_index != MAX_MFN_ALLOC ) mfn_list[alloc_index++] = mfn; - else - BUG_ON(HYPERVISOR_memory_op(XENMEM_decrease_reservation, - &reservation) != 1); + else { + int ret; + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, + &reservation); + BUG_ON(ret != 1); + } spin_unlock_irqrestore(&mfn_lock, flags); } #endif @@ -159,13 +162,15 @@ int netif_be_start_xmit(struct sk_buff * */ if (skb_shared(skb) || skb_cloned(skb) || !is_xen_skb(skb)) { int hlen = skb->data - skb->head; + int ret; struct sk_buff *nskb = dev_alloc_skb(hlen + skb->len); if ( unlikely(nskb == NULL) ) goto drop; skb_reserve(nskb, hlen); __skb_put(nskb, skb->len); - BUG_ON(skb_copy_bits(skb, -hlen, nskb->data - hlen, - skb->len + hlen)); + ret = skb_copy_bits(skb, -hlen, nskb->data - hlen, + skb->len + hlen); + BUG_ON(ret); nskb->dev = skb->dev; nskb->proto_csum_valid = skb->proto_csum_valid; dev_kfree_skb(skb); @@ -218,6 +223,7 @@ static void net_rx_action(unsigned long struct sk_buff *skb; u16 notify_list[NETIF_RX_RING_SIZE]; int notify_nr = 0; + int ret; skb_queue_head_init(&rxq); @@ -279,7 +285,8 @@ static void net_rx_action(unsigned long mcl++; mcl[-2].args[MULTI_UVMFLAGS_INDEX] = UVMF_TLB_FLUSH|UVMF_ALL; - BUG_ON(HYPERVISOR_multicall(rx_mcl, mcl - rx_mcl) != 0); + ret = HYPERVISOR_multicall(rx_mcl, mcl - rx_mcl); + BUG_ON(ret != 0); mcl = rx_mcl; if( HYPERVISOR_grant_table_op(GNTTABOP_transfer, grant_rx_op, @@ -421,6 +428,7 @@ inline static void net_tx_action_dealloc u16 pending_idx; PEND_RING_IDX dc, dp; netif_t *netif; + int ret; dc = dealloc_cons; dp = dealloc_prod; @@ -436,8 +444,9 @@ inline static void net_tx_action_dealloc gop->handle = grant_tx_ref[pending_idx]; gop++; } - BUG_ON(HYPERVISOR_grant_table_op( - GNTTABOP_unmap_grant_ref, tx_unmap_ops, gop - tx_unmap_ops)); + ret = HYPERVISOR_grant_table_op( + GNTTABOP_unmap_grant_ref, tx_unmap_ops, gop - tx_unmap_ops); + BUG_ON(ret); while (dealloc_cons != dp) { pending_idx = dealloc_ring[MASK_PEND_IDX(dealloc_cons++)]; @@ -477,6 +486,7 @@ static void net_tx_action(unsigned long NETIF_RING_IDX i; gnttab_map_grant_ref_t *mop; unsigned int data_len; + int ret; if (dealloc_cons != dealloc_prod) net_tx_action_dealloc(); @@ -599,8 +609,9 @@ static void net_tx_action(unsigned long if (mop == tx_map_ops) return; - BUG_ON(HYPERVISOR_grant_table_op( - GNTTABOP_map_grant_ref, tx_map_ops, mop - tx_map_ops)); + ret = HYPERVISOR_grant_table_op( + GNTTABOP_map_grant_ref, tx_map_ops, mop - tx_map_ops); + BUG_ON(ret); mop = tx_map_ops; while ((skb = __skb_dequeue(&tx_queue)) != NULL) { diff -purN linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c --- linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c 2005-10-22 10:13:08.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c 2005-10-27 11:28:17.000000000 +0200 @@ -78,6 +78,7 @@ tpmif_find(domid_t domid, long int insta static int map_frontend_page(tpmif_t *tpmif, unsigned long shared_page) { + int ret; struct gnttab_map_grant_ref op = { .host_addr = (unsigned long)tpmif->tx_area->addr, .flags = GNTMAP_host_map, @@ -86,8 +87,9 @@ map_frontend_page(tpmif_t *tpmif, unsign }; lock_vm_area(tpmif->tx_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); unlock_vm_area(tpmif->tx_area); + BUG_ON(ret); if (op.handle < 0) { DPRINTK(" Grant table operation failure !\n"); @@ -104,14 +106,16 @@ static void unmap_frontend_page(tpmif_t *tpmif) { struct gnttab_unmap_grant_ref op; + int ret; op.host_addr = (unsigned long)tpmif->tx_area->addr; op.handle = tpmif->shmem_handle; op.dev_bus_addr = 0; lock_vm_area(tpmif->tx_area); - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); unlock_vm_area(tpmif->tx_area); + BUG_ON(ret); } int diff -purN linux-2.6-xen-sparse/drivers/xen/util.c linux-2.6-xen-sparse/drivers/xen/util.c --- linux-2.6-xen-sparse/drivers/xen/util.c 2005-10-22 10:13:14.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/util.c 2005-10-27 11:28:49.000000000 +0200 @@ -34,7 +34,9 @@ struct vm_struct *alloc_vm_area(unsigned void free_vm_area(struct vm_struct *area) { - BUG_ON(remove_vm_area(area->addr) != area); + struct vm_struct *ret; + ret = remove_vm_area(area->addr); + BUG_ON(ret != area); kfree(area); } diff -purN linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c --- linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c 2005-10-22 10:13:18.000000000 +0200 +++ linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c 2005-10-27 11:29:33.000000000 +0200 @@ -737,6 +737,7 @@ static int __init xenbus_probe_init(void unsigned long page; evtchn_op_t op = { 0 }; + int ret; /* Allocate page. */ @@ -757,7 +758,8 @@ static int __init xenbus_probe_init(void op.u.alloc_unbound.dom = DOMID_SELF; op.u.alloc_unbound.remote_dom = 0; - BUG_ON(HYPERVISOR_event_channel_op(&op)); + ret = HYPERVISOR_event_channel_op(&op); + BUG_ON(ret); xen_start_info->store_evtchn = op.u.alloc_unbound.port; /* And finally publish the above info in /proc/xen */ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Muli Ben-Yehuda
2005-Oct-27 17:04 UTC
Re: [Xen-devel] [patch] Lets not put statements with side-effects inside BUG_ON()
On Thu, Oct 27, 2005 at 12:29:12PM +0200, Arjan van de Ven wrote:> BUG_ON() is a macro very very similar to assert(), and it''s a really bad > idea in general to put statements with side-effects inside such a construct > (for example the BUG_ON() could be compiled away for non-debug builds). > > The patch below fixes this for the drivers/xen tree.I''d like to see this applied, to both Xen and the vanilla kernel; however, it should be noted that no version of Linux or Xen compiles BUG_ON() away without evaluating its arguments. Cheers, Muli -- Muli Ben-Yehuda http://www.mulix.org | http://mulix.livejournal.com/ _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Arjan van de Ven
2005-Oct-27 17:06 UTC
Re: [Xen-devel] [patch] Lets not put statements with side-effects inside BUG_ON()
On Thu, Oct 27, 2005 at 07:04:01PM +0200, Muli Ben-Yehuda wrote:> On Thu, Oct 27, 2005 at 12:29:12PM +0200, Arjan van de Ven wrote: > > > BUG_ON() is a macro very very similar to assert(), and it''s a really bad > > idea in general to put statements with side-effects inside such a construct > > (for example the BUG_ON() could be compiled away for non-debug builds). > > > > The patch below fixes this for the drivers/xen tree. > > I''d like to see this applied, to both Xen and the vanilla kernel; > however, it should be noted that no version of Linux or Xen compiles > BUG_ON() away without evaluating its arguments.but it should be able to ;) yeah I agree it''s not a hard bug per se, but something that should be done anyway ;) _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Kip Macy
2005-Oct-27 17:49 UTC
Re: [Xen-devel] [patch] Lets not put statements with side-effects inside BUG_ON()
The intention is to panic if an operation that should *never* fail (debug or release) does in fact fail. It isn't debug scaffolding like an assert. -Kip On 10/27/05, Arjan van de Ven <arjanv@redhat.com> wrote:> > Hi, > > BUG_ON() is a macro very very similar to assert(), and it's a really bad > idea in general to put statements with side-effects inside such a > construct > (for example the BUG_ON() could be compiled away for non-debug builds). > > The patch below fixes this for the drivers/xen tree. > > I chose the general construct to store the respective return values in a > local var instead of an if (...) BUG(); because it looks like most of > these > BUG_ON()'s need to be replaced by real error handling when going towards > production releases anyway, and then the local variable is going to be > needed anyway. (and besides the if construct tends to get unreable due to > the unlikely() etc). > > Please apply. > > Greetings, > Arjan van de Ven > > > diff -purN linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c > linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c > --- linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c 2005-10-26 17:13: > 26.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/balloon/balloon.c 2005-10-27 11:26: > 28.000000000 +0200 > @@ -191,12 +191,13 @@ static int increase_reservation(unsigned > rc = HYPERVISOR_memory_op( > XENMEM_increase_reservation, &reservation); > if (rc < nr_pages) { > + int ret; > /* We hit the Xen hard limit: reprobe. */ > reservation.extent_start = mfn_list; > reservation.nr_extents = rc; > - BUG_ON(HYPERVISOR_memory_op( > - XENMEM_decrease_reservation, > - &reservation) != rc); > + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, > + &reservation); > + BUG_ON(ret != rc); > hard_limit = current_pages + rc - driver_pages; > goto out; > } > @@ -213,11 +214,14 @@ static int increase_reservation(unsigned > xen_machphys_update(mfn_list[i], pfn); > > /* Link back into the page tables if not highmem. */ > - if (pfn < max_low_pfn) > - BUG_ON(HYPERVISOR_update_va_mapping( > + if (pfn < max_low_pfn) { > + int ret; > + ret = HYPERVISOR_update_va_mapping( > (unsigned long)__va(pfn << PAGE_SHIFT), > pfn_pte_ma(mfn_list[i], PAGE_KERNEL), > - 0)); > + 0); > + BUG_ON(ret); > + } > > /* Relinquish the page back to the allocator. */ > ClearPageReserved(page); > @@ -242,6 +246,7 @@ static int decrease_reservation(unsigned > struct page *page; > void *v; > int need_sleep = 0; > + int ret; > struct xen_memory_reservation reservation = { > .address_bits = 0, > .extent_order = 0, > @@ -268,8 +273,9 @@ static int decrease_reservation(unsigned > if (!PageHighMem(page)) { > v = phys_to_virt(pfn << PAGE_SHIFT); > scrub_pages(v, 1); > - BUG_ON(HYPERVISOR_update_va_mapping( > - (unsigned long)v, __pte_ma(0), 0)); > + ret = HYPERVISOR_update_va_mapping( > + (unsigned long)v, __pte_ma(0), 0); > + BUG_ON(ret); > } > #ifdef CONFIG_XEN_SCRUB_PAGES > else { > @@ -295,8 +301,8 @@ static int decrease_reservation(unsigned > > reservation.extent_start = mfn_list; > reservation.nr_extents = nr_pages; > - BUG_ON(HYPERVISOR_memory_op( > - XENMEM_decrease_reservation, &reservation) != nr_pages); > + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation); > + BUG_ON(ret != nr_pages); > > current_pages -= nr_pages; > totalram_pages = current_pages; > @@ -501,6 +507,7 @@ static int dealloc_pte_fn( > pte_t *pte, struct page *pte_page, unsigned long addr, void *data) > { > unsigned long mfn = pte_mfn(*pte); > + int ret; > struct xen_memory_reservation reservation = { > .extent_start = &mfn, > .nr_extents = 1, > @@ -510,8 +517,8 @@ static int dealloc_pte_fn( > set_pte_at(&init_mm, addr, pte, __pte_ma(0)); > phys_to_machine_mapping[__pa(addr) >> PAGE_SHIFT] > INVALID_P2M_ENTRY; > - BUG_ON(HYPERVISOR_memory_op( > - XENMEM_decrease_reservation, &reservation) != 1); > + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, &reservation); > + BUG_ON(ret != 1); > return 0; > } > > @@ -519,6 +526,7 @@ struct page *balloon_alloc_empty_page_ra > { > unsigned long vstart, flags; > unsigned int order = get_order(nr_pages * PAGE_SIZE); > + int ret; > > vstart = __get_free_pages(GFP_KERNEL, order); > if (vstart == 0) > @@ -527,8 +535,9 @@ struct page *balloon_alloc_empty_page_ra > scrub_pages(vstart, 1 << order); > > balloon_lock(flags); > - BUG_ON(generic_page_range( > - &init_mm, vstart, PAGE_SIZE << order, dealloc_pte_fn, NULL)); > + ret = generic_page_range( > + &init_mm, vstart, PAGE_SIZE << order, dealloc_pte_fn, NULL); > + BUG_ON(ret); > current_pages -= 1UL << order; > balloon_unlock(flags); > > diff -purN linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c > linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c > --- linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c 2005-10-22 10:12: > 23.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/blkback/blkback.c 2005-10-27 11:16: > 54.000000000 +0200 > @@ -108,6 +108,7 @@ static void fast_flush_area(int idx, int > struct gnttab_unmap_grant_ref unmap[BLKIF_MAX_SEGMENTS_PER_REQUEST]; > unsigned int i, invcount = 0; > u16 handle; > + int ret; > > for (i = 0; i < nr_pages; i++) { > handle = pending_handle(idx, i); > @@ -120,8 +121,9 @@ static void fast_flush_area(int idx, int > invcount++; > } > > - BUG_ON(HYPERVISOR_grant_table_op( > - GNTTABOP_unmap_grant_ref, unmap, invcount)); > + ret = HYPERVISOR_grant_table_op( > + GNTTABOP_unmap_grant_ref, unmap, invcount); > + BUG_ON(ret); > } > > > @@ -338,6 +340,7 @@ static void dispatch_rw_block_io(blkif_t > struct bio *bio = NULL, *biolist[BLKIF_MAX_SEGMENTS_PER_REQUEST]; > int nbio = 0; > request_queue_t *q; > + int ret; > > /* Check that number of segments is sane. */ > nseg = req->nr_segments; > @@ -367,8 +370,8 @@ static void dispatch_rw_block_io(blkif_t > map[i].flags |= GNTMAP_readonly; > } > > - BUG_ON(HYPERVISOR_grant_table_op( > - GNTTABOP_map_grant_ref, map, nseg)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, nseg); > + BUG_ON(ret); > > for (i = 0; i < nseg; i++) { > if (unlikely(map[i].handle < 0)) { > @@ -493,6 +496,7 @@ static int __init blkif_init(void) > { > int i; > struct page *page; > + int ret; > > blkif_interface_init(); > > @@ -509,7 +513,8 @@ static int __init blkif_init(void) > spin_lock_init(&blkio_schedule_list_lock); > INIT_LIST_HEAD(&blkio_schedule_list); > > - BUG_ON(kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES) < 0); > + ret = kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES); > + BUG_ON(ret < 0); > > blkif_xenbus_init(); > > diff -purN linux-2.6-xen-sparse/drivers/xen/blkback/interface.c > linux-2.6-xen-sparse/drivers/xen/blkback/interface.c > --- linux-2.6-xen-sparse/drivers/xen/blkback/interface.c 2005-10-22 10:12: > 24.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/blkback/interface.c 2005-10-27 11:17: > 29.000000000 +0200 > @@ -31,6 +31,7 @@ blkif_t *alloc_blkif(domid_t domid) > static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) > { > struct gnttab_map_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)blkif->blk_ring_area->addr; > op.flags = GNTMAP_host_map; > @@ -38,8 +39,9 @@ static int map_frontend_page(blkif_t *bl > op.dom = blkif->domid; > > lock_vm_area(blkif->blk_ring_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); > unlock_vm_area(blkif->blk_ring_area); > + BUG_ON(ret); > > if (op.handle < 0) { > DPRINTK(" Grant table operation failure !\n"); > @@ -55,14 +57,16 @@ static int map_frontend_page(blkif_t *bl > static void unmap_frontend_page(blkif_t *blkif) > { > struct gnttab_unmap_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)blkif->blk_ring_area->addr; > op.handle = blkif->shmem_handle; > op.dev_bus_addr = 0; > > lock_vm_area(blkif->blk_ring_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); > unlock_vm_area(blkif->blk_ring_area); > + BUG_ON(ret); > } > > int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int > evtchn) > diff -purN linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c > linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c > --- linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c 2005-10-22 10:12: > 27.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/blkfront/blkfront.c 2005-10-27 11:15: > 41.000000000 +0200 > @@ -305,6 +305,7 @@ static irqreturn_t blkif_int(int irq, vo > > for (i = info->ring.rsp_cons; i != rp; i++) { > unsigned long id; > + int ret; > > bret = RING_GET_RESPONSE(&info->ring, i); > id = bret->id; > @@ -321,9 +322,10 @@ static irqreturn_t blkif_int(int irq, vo > DPRINTK("Bad return from blkdev data " > "request: %x\n", bret->status); > > - BUG_ON(end_that_request_first( > + ret = end_that_request_first( > req, (bret->status == BLKIF_RSP_OKAY), > - req->hard_nr_sectors)); > + req->hard_nr_sectors); > + BUG_ON(ret); > end_that_request_last(req); > break; > default: > diff -purN linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c > linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c > --- linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c 2005-10-22 10:12: > 30.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/blktap/blktap.c 2005-10-27 11:19: > 42.000000000 +0200 > @@ -413,6 +413,7 @@ static void fast_flush_area(int idx, int > unsigned int i, op = 0; > struct grant_handle_pair *handle; > unsigned long ptep; > + int ret; > > for ( i = 0; i < nr_pages; i++) > { > @@ -440,8 +441,8 @@ static void fast_flush_area(int idx, int > BLKTAP_INVALIDATE_HANDLE(handle); > } > > - BUG_ON(HYPERVISOR_grant_table_op( > - GNTTABOP_unmap_grant_ref, unmap, op)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, unmap, op); > + BUG_ON(ret); > > if (blktap_vma != NULL) > zap_page_range(blktap_vma, > @@ -673,6 +674,7 @@ static void dispatch_rw_block_io(blkif_t > struct gnttab_map_grant_ref map[BLKIF_MAX_SEGMENTS_PER_REQUEST*2]; > int op, ret; > unsigned int nseg; > + int retval; > > /* Check that number of segments is sane. */ > nseg = req->nr_segments; > @@ -740,8 +742,8 @@ static void dispatch_rw_block_io(blkif_t > op++; > } > > - BUG_ON(HYPERVISOR_grant_table_op( > - GNTTABOP_map_grant_ref, map, op)); > + retval = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, map, op); > + BUG_ON(retval); > > op = 0; > for (i = 0; i < (req->nr_segments*2); i += 2) { > @@ -877,7 +879,8 @@ static int __init blkif_init(void) > spin_lock_init(&blkio_schedule_list_lock); > INIT_LIST_HEAD(&blkio_schedule_list); > > - BUG_ON(kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES) < 0); > + i = kernel_thread(blkio_schedule, 0, CLONE_FS | CLONE_FILES); > + BUG_ON(i<0); > > blkif_xenbus_init(); > > diff -purN linux-2.6-xen-sparse/drivers/xen/blktap/interface.c > linux-2.6-xen-sparse/drivers/xen/blktap/interface.c > --- linux-2.6-xen-sparse/drivers/xen/blktap/interface.c 2005-10-22 10:12: > 31.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/blktap/interface.c 2005-10-27 11:20: > 19.000000000 +0200 > @@ -31,6 +31,7 @@ blkif_t *alloc_blkif(domid_t domid) > static int map_frontend_page(blkif_t *blkif, unsigned long shared_page) > { > struct gnttab_map_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)blkif->blk_ring_area->addr; > op.flags = GNTMAP_host_map; > @@ -38,8 +39,9 @@ static int map_frontend_page(blkif_t *bl > op.dom = blkif->domid; > > lock_vm_area(blkif->blk_ring_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); > unlock_vm_area(blkif->blk_ring_area); > + BUG_ON(ret); > > if (op.handle < 0) { > DPRINTK(" Grant table operation failure !\n"); > @@ -55,14 +57,16 @@ static int map_frontend_page(blkif_t *bl > static void unmap_frontend_page(blkif_t *blkif) > { > struct gnttab_unmap_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)blkif->blk_ring_area->addr; > op.handle = blkif->shmem_handle; > op.dev_bus_addr = 0; > > lock_vm_area(blkif->blk_ring_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); > unlock_vm_area(blkif->blk_ring_area); > + BUG_ON(ret); > } > > int blkif_map(blkif_t *blkif, unsigned long shared_page, unsigned int > evtchn) > diff -purN linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c > linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c > --- linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c 2005-10-22 10:12: > 37.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/evtchn/evtchn.c 2005-10-27 11:27: > 29.000000000 +0200 > @@ -282,6 +282,7 @@ static int evtchn_ioctl(struct inode *in > > case IOCTL_EVTCHN_UNBIND: { > struct ioctl_evtchn_unbind unbind; > + int ret; > > rc = -EFAULT; > if (copy_from_user(&unbind, (void *)arg, sizeof(unbind))) > @@ -306,7 +307,8 @@ static int evtchn_ioctl(struct inode *in > > op.cmd = EVTCHNOP_close; > op.u.close.port = unbind.port; > - BUG_ON(HYPERVISOR_event_channel_op(&op)); > + ret = HYPERVISOR_event_channel_op(&op); > + BUG_ON(ret); > > rc = 0; > break; > @@ -399,6 +401,7 @@ static int evtchn_release(struct inode * > > for (i = 0; i < NR_EVENT_CHANNELS; i++) > { > + int ret; > if (port_user[i] != u) > continue; > > @@ -407,7 +410,8 @@ static int evtchn_release(struct inode * > > op.cmd = EVTCHNOP_close; > op.u.close.port = i; > - BUG_ON(HYPERVISOR_event_channel_op(&op)); > + ret = HYPERVISOR_event_channel_op(&op); > + BUG_ON(ret); > } > > spin_unlock_irq(&port_user_lock); > diff -purN linux-2.6-xen-sparse/drivers/xen/netback/interface.c > linux-2.6-xen-sparse/drivers/xen/netback/interface.c > --- linux-2.6-xen-sparse/drivers/xen/netback/interface.c 2005-10-22 10:12: > 38.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/netback/interface.c 2005-10-27 11:10: > 20.000000000 +0200 > @@ -115,6 +115,7 @@ static int map_frontend_pages( > netif_t *netif, grant_ref_t tx_ring_ref, grant_ref_t rx_ring_ref) > { > struct gnttab_map_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)netif->comms_area->addr; > op.flags = GNTMAP_host_map; > @@ -122,8 +123,9 @@ static int map_frontend_pages( > op.dom = netif->domid; > > lock_vm_area(netif->comms_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); > unlock_vm_area(netif->comms_area); > + BUG_ON(ret); > > if (op.handle < 0) { > DPRINTK(" Gnttab failure mapping tx_ring_ref!\n"); > @@ -139,8 +141,9 @@ static int map_frontend_pages( > op.dom = netif->domid; > > lock_vm_area(netif->comms_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); > unlock_vm_area(netif->comms_area); > + BUG_ON(ret); > > if (op.handle < 0) { > DPRINTK(" Gnttab failure mapping rx_ring_ref!\n"); > @@ -156,22 +159,25 @@ static int map_frontend_pages( > static void unmap_frontend_pages(netif_t *netif) > { > struct gnttab_unmap_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)netif->comms_area->addr; > op.handle = netif->tx_shmem_handle; > op.dev_bus_addr = 0; > > lock_vm_area(netif->comms_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); > unlock_vm_area(netif->comms_area); > + BUG_ON(ret); > > op.host_addr = (unsigned long)netif->comms_area->addr + PAGE_SIZE; > op.handle = netif->rx_shmem_handle; > op.dev_bus_addr = 0; > > lock_vm_area(netif->comms_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); > unlock_vm_area(netif->comms_area); > + BUG_ON(ret); > } > > int netif_map(netif_t *netif, unsigned long tx_ring_ref, > diff -purN linux-2.6-xen-sparse/drivers/xen/netback/netback.c > linux-2.6-xen-sparse/drivers/xen/netback/netback.c > --- linux-2.6-xen-sparse/drivers/xen/netback/netback.c 2005-10-22 10:12: > 39.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/netback/netback.c 2005-10-27 11:12: > 42.000000000 +0200 > @@ -112,9 +112,12 @@ static void free_mfn(unsigned long mfn) > spin_lock_irqsave(&mfn_lock, flags); > if ( alloc_index != MAX_MFN_ALLOC ) > mfn_list[alloc_index++] = mfn; > - else > - BUG_ON(HYPERVISOR_memory_op(XENMEM_decrease_reservation, > - &reservation) != 1); > + else { > + int ret; > + ret = HYPERVISOR_memory_op(XENMEM_decrease_reservation, > + &reservation); > + BUG_ON(ret != 1); > + } > spin_unlock_irqrestore(&mfn_lock, flags); > } > #endif > @@ -159,13 +162,15 @@ int netif_be_start_xmit(struct sk_buff * > */ > if (skb_shared(skb) || skb_cloned(skb) || !is_xen_skb(skb)) { > int hlen = skb->data - skb->head; > + int ret; > struct sk_buff *nskb = dev_alloc_skb(hlen + skb->len); > if ( unlikely(nskb == NULL) ) > goto drop; > skb_reserve(nskb, hlen); > __skb_put(nskb, skb->len); > - BUG_ON(skb_copy_bits(skb, -hlen, nskb->data - hlen, > - skb->len + hlen)); > + ret = skb_copy_bits(skb, -hlen, nskb->data - hlen, > + skb->len + hlen); > + BUG_ON(ret); > nskb->dev = skb->dev; > nskb->proto_csum_valid = skb->proto_csum_valid; > dev_kfree_skb(skb); > @@ -218,6 +223,7 @@ static void net_rx_action(unsigned long > struct sk_buff *skb; > u16 notify_list[NETIF_RX_RING_SIZE]; > int notify_nr = 0; > + int ret; > > skb_queue_head_init(&rxq); > > @@ -279,7 +285,8 @@ static void net_rx_action(unsigned long > mcl++; > > mcl[-2].args[MULTI_UVMFLAGS_INDEX] = UVMF_TLB_FLUSH|UVMF_ALL; > - BUG_ON(HYPERVISOR_multicall(rx_mcl, mcl - rx_mcl) != 0); > + ret = HYPERVISOR_multicall(rx_mcl, mcl - rx_mcl); > + BUG_ON(ret != 0); > > mcl = rx_mcl; > if( HYPERVISOR_grant_table_op(GNTTABOP_transfer, grant_rx_op, > @@ -421,6 +428,7 @@ inline static void net_tx_action_dealloc > u16 pending_idx; > PEND_RING_IDX dc, dp; > netif_t *netif; > + int ret; > > dc = dealloc_cons; > dp = dealloc_prod; > @@ -436,8 +444,9 @@ inline static void net_tx_action_dealloc > gop->handle = grant_tx_ref[pending_idx]; > gop++; > } > - BUG_ON(HYPERVISOR_grant_table_op( > - GNTTABOP_unmap_grant_ref, tx_unmap_ops, gop - tx_unmap_ops)); > + ret = HYPERVISOR_grant_table_op( > + GNTTABOP_unmap_grant_ref, tx_unmap_ops, gop - tx_unmap_ops); > + BUG_ON(ret); > > while (dealloc_cons != dp) { > pending_idx = dealloc_ring[MASK_PEND_IDX(dealloc_cons++)]; > @@ -477,6 +486,7 @@ static void net_tx_action(unsigned long > NETIF_RING_IDX i; > gnttab_map_grant_ref_t *mop; > unsigned int data_len; > + int ret; > > if (dealloc_cons != dealloc_prod) > net_tx_action_dealloc(); > @@ -599,8 +609,9 @@ static void net_tx_action(unsigned long > if (mop == tx_map_ops) > return; > > - BUG_ON(HYPERVISOR_grant_table_op( > - GNTTABOP_map_grant_ref, tx_map_ops, mop - tx_map_ops)); > + ret = HYPERVISOR_grant_table_op( > + GNTTABOP_map_grant_ref, tx_map_ops, mop - tx_map_ops); > + BUG_ON(ret); > > mop = tx_map_ops; > while ((skb = __skb_dequeue(&tx_queue)) != NULL) { > diff -purN linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c > linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c > --- linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c 2005-10-22 10:13: > 08.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/tpmback/interface.c 2005-10-27 11:28: > 17.000000000 +0200 > @@ -78,6 +78,7 @@ tpmif_find(domid_t domid, long int insta > static int > map_frontend_page(tpmif_t *tpmif, unsigned long shared_page) > { > + int ret; > struct gnttab_map_grant_ref op = { > .host_addr = (unsigned long)tpmif->tx_area->addr, > .flags = GNTMAP_host_map, > @@ -86,8 +87,9 @@ map_frontend_page(tpmif_t *tpmif, unsign > }; > > lock_vm_area(tpmif->tx_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1); > unlock_vm_area(tpmif->tx_area); > + BUG_ON(ret); > > if (op.handle < 0) { > DPRINTK(" Grant table operation failure !\n"); > @@ -104,14 +106,16 @@ static void > unmap_frontend_page(tpmif_t *tpmif) > { > struct gnttab_unmap_grant_ref op; > + int ret; > > op.host_addr = (unsigned long)tpmif->tx_area->addr; > op.handle = tpmif->shmem_handle; > op.dev_bus_addr = 0; > > lock_vm_area(tpmif->tx_area); > - BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1)); > + ret = HYPERVISOR_grant_table_op(GNTTABOP_unmap_grant_ref, &op, 1); > unlock_vm_area(tpmif->tx_area); > + BUG_ON(ret); > } > > int > diff -purN linux-2.6-xen-sparse/drivers/xen/util.c linux-2.6-xen-sparse > /drivers/xen/util.c > --- linux-2.6-xen-sparse/drivers/xen/util.c 2005-10-22 10:13:14.000000000+0200 > +++ linux-2.6-xen-sparse/drivers/xen/util.c 2005-10-27 11:28:49.000000000+0200 > @@ -34,7 +34,9 @@ struct vm_struct *alloc_vm_area(unsigned > > void free_vm_area(struct vm_struct *area) > { > - BUG_ON(remove_vm_area(area->addr) != area); > + struct vm_struct *ret; > + ret = remove_vm_area(area->addr); > + BUG_ON(ret != area); > kfree(area); > } > > diff -purN linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c > linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c > --- linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c 2005-10-22 > 10:13:18.000000000 +0200 > +++ linux-2.6-xen-sparse/drivers/xen/xenbus/xenbus_probe.c 2005-10-27 > 11:29:33.000000000 +0200 > @@ -737,6 +737,7 @@ static int __init xenbus_probe_init(void > > unsigned long page; > evtchn_op_t op = { 0 }; > + int ret; > > > /* Allocate page. */ > @@ -757,7 +758,8 @@ static int __init xenbus_probe_init(void > op.u.alloc_unbound.dom = DOMID_SELF; > op.u.alloc_unbound.remote_dom = 0; > > - BUG_ON(HYPERVISOR_event_channel_op(&op)); > + ret = HYPERVISOR_event_channel_op(&op); > + BUG_ON(ret); > xen_start_info->store_evtchn = op.u.alloc_unbound.port; > > /* And finally publish the above info in /proc/xen */ > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel