lkml.org 
[lkml]   [2004]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] NMI trigger switch support for debugging
AKIYAMA Nobuyuki writes:
> +int unknown_nmi_panic = 0;

It's a kernel coding standard to _not_ explicitly initialise
static-extent data to zero.

> +/*
> + * proc handler for /proc/sys/kernel/unknown_nmi_panic
> + */
> +int proc_unknown_nmi_panic(ctl_table *table, int write,
> + struct file *file, void __user *buffer, size_t *length)
> +{
> + int old_state;
> +
> + old_state = unknown_nmi_panic;
> + proc_dointvec(table, write, file, buffer, length);
> + if (!old_state == !unknown_nmi_panic)
> + return 0;

This conditional looks terribly obscure.
Can you simplify it or explain your intention here?

> + if (unknown_nmi_panic) {
> + if (reserve_lapic_nmi() < 0) {
> + unknown_nmi_panic = 0;
> + return -EBUSY;
> + } else {
> + set_nmi_callback(unknown_nmi_panic_callback);
> + }
> + } else {
> + release_lapic_nmi();

You're invoking release_lapic_nmi() in response to user
input, without having verified that _you_ had done a
reserve_lapic_nmi() before.

It looks like the code will do horrible things if the
operator invokes the sysctl incorrectly. Such errors do
happen, so code should include basic sanity checks.

/Mikael
-
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:03    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site