lkml.org 
[lkml]   [2004]   [Aug]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [2.6 patch][2/3] kernel/ BUG -> BUG_ON conversions
    Date
    On Aug 28, 2004, at 11:17, Adrian Bunk wrote:
    > The patch below does BUG -> BUG_ON conversions in kernel/ .
    >
    > --- linux-2.6.9-rc1-mm1-full-3.4/kernel/cpu.c.old 2004-08-28
    > 16:05:17.000000000 +0200
    > +++ linux-2.6.9-rc1-mm1-full-3.4/kernel/cpu.c 2004-08-28
    > 16:06:24.000000000 +0200
    > @@ -156,9 +156,8 @@
    > kthread_bind(p, smp_processor_id());
    >
    > /* CPU is completely dead: tell everyone. Too late to complain. */
    > - if (notifier_call_chain(&cpu_chain, CPU_DEAD, (void *)(long)cpu)
    > - == NOTIFY_BAD)
    > - BUG();
    > + BUG_ON(notifier_call_chain(&cpu_chain, CPU_DEAD, (void *)(long)cpu)
    > + == NOTIFY_BAD);

    Can't do this, without DEBUG the notifier_call_chain _won't_ be executed

    > check_for_tasks(cpu);
    >
    > @@ -203,8 +202,7 @@
    > ret = __cpu_up(cpu);
    > if (ret != 0)
    > goto out_notify;
    > - if (!cpu_online(cpu))
    > - BUG();
    > + BUG_ON(!cpu_online(cpu));

    Does cpu_online() have any side effects?

    >
    > /* Now call notifier in preparation. */
    > notifier_call_chain(&cpu_chain, CPU_ONLINE, hcpu);
    > --- linux-2.6.9-rc1-mm1-full-3.4/kernel/exit.c.old 2004-08-28
    > 16:05:17.000000000 +0200
    > +++ linux-2.6.9-rc1-mm1-full-3.4/kernel/exit.c 2004-08-28
    > 16:08:24.000000000 +0200
    > @@ -503,7 +503,7 @@
    > down_read(&mm->mmap_sem);
    > }
    > atomic_inc(&mm->mm_count);
    > - if (mm != tsk->active_mm) BUG();
    > + BUG_ON(mm != tsk->active_mm);
    > /* more a memory barrier than a real lock */
    > task_lock(tsk);
    > tsk->mm = NULL;
    > @@ -878,11 +877,9 @@
    > const struct list_head *tmp, *head = &link->pidptr->task_list;
    >
    > #ifdef CONFIG_SMP
    > - if (!p->sighand)
    > - BUG();
    > - if (!spin_is_locked(&p->sighand->siglock) &&
    > - !rwlock_is_locked(&tasklist_lock))
    > - BUG();
    > + BUG_ON(!p->sighand);
    > + BUG_ON(!spin_is_locked(&p->sighand->siglock) &&
    > + !rwlock_is_locked(&tasklist_lock));

    These _should_ be OK, but I'm not sure if spin_is_locked and
    rwlock_is_locked
    have any memory barrier guarantees.

    > #endif
    > tmp = link->pid_chain.next;
    > if (tmp == head)
    > @@ -1350,8 +1347,7 @@
    > if (options & __WNOTHREAD)
    > break;
    > tsk = next_thread(tsk);
    > - if (tsk->signal != current->signal)
    > - BUG();
    > + BUG_ON(tsk->signal != current->signal);
    > } while (tsk != current);
    >
    > read_unlock(&tasklist_lock);
    > --- linux-2.6.9-rc1-mm1-full-3.4/kernel/fork.c.old 2004-08-28
    > 16:05:17.000000000 +0200
    > +++ linux-2.6.9-rc1-mm1-full-3.4/kernel/fork.c 2004-08-28
    > 16:08:45.000000000 +0200
    > @@ -809,8 +809,7 @@
    > struct files_struct *files = current->files;
    > int rc;
    >
    > - if(!files)
    > - BUG();
    > + BUG_ON(!files);
    >
    > /* This can race but the race causes us to copy when we don't
    > need to and drop the copy */
    > --- linux-2.6.9-rc1-mm1-full-3.4/kernel/module.c.old 2004-08-28
    > 16:05:17.000000000 +0200
    > +++ linux-2.6.9-rc1-mm1-full-3.4/kernel/module.c 2004-08-28
    > 16:13:44.000000000 +0200
    > @@ -655,8 +655,7 @@
    > const unsigned long *crc;
    >
    > spin_lock_irqsave(&modlist_lock, flags);
    > - if (!__find_symbol(symbol, &owner, &crc, 1))
    > - BUG();
    > + BUG_ON(!__find_symbol(symbol, &owner, &crc, 1));

    Does __find_symbol have side effects?

    > module_put(owner);
    > spin_unlock_irqrestore(&modlist_lock, flags);
    > }
    > @@ -667,8 +666,7 @@
    > unsigned long flags;
    >
    > spin_lock_irqsave(&modlist_lock, flags);
    > - if (!kernel_text_address((unsigned long)addr))
    > - BUG();
    > + BUG_ON(!kernel_text_address((unsigned long)addr));

    Side effects?

    > module_put(module_text_address((unsigned long)addr));
    > spin_unlock_irqrestore(&modlist_lock, flags);
    > @@ -905,8 +903,7 @@
    > const unsigned long *crc;
    > struct module *owner;
    >
    > - if (!__find_symbol("struct_module", &owner, &crc, 1))
    > - BUG();
    > + BUG_ON(!__find_symbol("struct_module", &owner, &crc, 1));
    > return check_version(sechdrs, versindex, "struct_module", mod,
    > crc);
    > }

    [Bunch of patch chomped]

    Please verify that you don't pull important function calls into
    BUG_ON() statements.

    Cheers,
    Kyle Moffett

    -----BEGIN GEEK CODE BLOCK-----
    Version: 3.12
    GCM/CS/IT/U d- s++: a17 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$
    L++++(+++) E W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+
    PGP+++ t+(+++) 5 X R? tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r
    !y?(-)
    ------END GEEK CODE BLOCK------


    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:05    [W:0.029 / U:60.436 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site