fix register_sysrq() in 2.4.9++

u> +

include/linux/sysrq.h line 94

Quoted 4 lines

include/linux/sysrq.h renamed from include/linux/sysrq.h.org +4 -0

+static inline int register_sysrq_key(int key, struct sysrq_key_op *op_p)
+{
+     return -1;
+}

make it report ok as other non compiled in stuff does - then you can avoid masses of ifdefs

Yeah, I considered that, and it doesn't matter to me whether it reports 0 or -1, but it's the data pointer that (mostly) requires the #ifdefs, unless the data is always present or a dummy data pointer is used.... ?

Quoted 5 lines

Yeah, I considered that, and it doesn't matter to me whether it reports 0 or -1, but it's the data pointer that (mostly) requires the #ifdefs, unless the data is always present or a dummy data pointer is used.... ?

#define it to an inline without some arguments ?

Quoted 7 lines · Alan Cox

Quoted 5 lines

Yeah, I considered that, and it doesn't matter to me whether it reports 0 or -1, but it's the data pointer that (mostly) requires the #ifdefs, unless the data is always present or a dummy data pointer is used.... ?

#define it to an inline without some arguments ?

~~~~~~~~~~~~~~~~~~
I can't get that to work, but someone else may be able to...

Here's another version for you to consider.

The [un]register_sysrq_key() calls return 0 when CONFIG_MAGIC_SYSRQ is not defined/configured.
However, it sacrifices one small data structure of 3 pointers.

~Randy

arch/i386/kernel/apm.c renamed from arch/i386/kernel/apm.c.org +2 -0

@@ -703,6 +703,8 @@
 	help_msg:       "Off",
 	action_msg:     "Power Off\n"
 };
+#else
+struct sysrq_key_op sysrq_poweroff_op;
 #endif
 
 

include/linux/sysrq.h renamed from include/linux/sysrq.h.org +11 -2

@@ -87,8 +87,17 @@
 }
 
 #else
-#define register_sysrq_key(a,b)		do {} while(0)
-#define unregister_sysrq_key(a,b)	do {} while(0)
+
+static inline int register_sysrq_key(int key, struct sysrq_key_op *op_p)
+{
+	return 0;
+}
+
+static inline int unregister_sysrq_key(int key, struct sysrq_key_op *op_p)
+{
+	return 0;
+}
+
 #endif
 
 /* Deferred actions */

I'm not sure if this is sufficient. The low level interfaces need to be exposed, and if we are not expecting modules to pay attention to the CONFIG_MAGIC_SYSRQ setting, then the all of these interfaces need to be overridden.

However, do we even need this #ifdef CONFIG_MAGIC_SYSRQ block at all? What does it matter if modules register or unregister events, if they cannot be called?

The old code only zaped the enable if sysrq was not defined, and that is what I'm doing in the table. Some real changes would be neccessary to actually drop out the whole system.

There is also no real reason to try and no-op these functions for speed, as they are trivial and FAR outside of the main call path.

So the way to go I see here is:
a) allow the registration functions to always be defined.
and either:
b) handle the return failure in the __sysrq_XXX functions themselves,
c) or not.

++ 20/09/01 11:57 -0700 - Randy.Dunlap:

Quoted 51 lines

Quoted 7 lines · Alan Cox

Quoted 5 lines

Yeah, I considered that, and it doesn't matter to me whether it reports 0 or -1, but it's the data pointer that (mostly) requires the #ifdefs, unless the data is always present or a dummy data pointer is used.... ?

#define it to an inline without some arguments ?

~~~~~~~~~~~~~~~~~~
I can't get that to work, but someone else may be able to...

Here's another version for you to consider.

The [un]register_sysrq_key() calls return 0 when CONFIG_MAGIC_SYSRQ is not defined/configured.
However, it sacrifices one small data structure of 3 pointers.

~Randy

arch/i386/kernel/apm.c renamed from arch/i386/kernel/apm.c.org +2 -0

@@ -703,6 +703,8 @@
 	help_msg:       "Off",
 	action_msg:     "Power Off\n"
 };
+#else
+struct sysrq_key_op sysrq_poweroff_op;
 #endif
 
 

include/linux/sysrq.h renamed from include/linux/sysrq.h.org +11 -2

@@ -87,8 +87,17 @@
 }
 
 #else
-#define register_sysrq_key(a,b)		do {} while(0)
-#define unregister_sysrq_key(a,b)	do {} while(0)
+
+static inline int register_sysrq_key(int key, struct sysrq_key_op *op_p)
+{
+	return 0;
+}
+
+static inline int unregister_sysrq_key(int key, struct sysrq_key_op *op_p)
+{
+	return 0;
+}
+
 #endif
 
 /* Deferred actions */

Signature · 8 lines

Crutcher        <[email protected]>
GCS d--- s+:>+:- a-- C++++$ UL++++$ L+++$>++++ !E PS+++ PE Y+ PGP+>++++
    R-(+++) !tv(+++) b+(++++) G+ e>++++ h+>++ r* y+>*$
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

++ 21/09/01 18:22 -0400 - Crutcher Dunnavant:

Quoted 13 lines

I'm not sure if this is sufficient. The low level interfaces need to be exposed, and if we are not expecting modules to pay attention to the CONFIG_MAGIC_SYSRQ setting, then the all of these interfaces need to be overridden.

However, do we even need this #ifdef CONFIG_MAGIC_SYSRQ block at all? What does it matter if modules register or unregister events, if they cannot be called?

The old code only zaped the enable if sysrq was not defined, and that is what I'm doing in the table. Some real changes would be neccessary to actually drop out the whole system.

Quoted 8 lines

There is also no real reason to try and no-op these functions for speed, as they are trivial and FAR outside of the main call path.

So the way to go I see here is:
a) allow the registration functions to always be defined.
and either:
b) handle the return failure in the __sysrq_XXX functions themselves,
c) or not.

A 'dont-close-it' patch is attached.

----------------------------------------------------------------------
Name: patch-2.4.10-pre13-sysrq_register
patch-2.4.10-pre13-sysrq_register Type: Plain Text (text/plain)
Description: patch-2.4.10-pre13-sysrq_register