unhappy with current.h

From: Daniele Lugli <[email protected]>
Date: Mon, 14 Oct 2002 21:46:08 +0200

Moral of the story: in my opinion kernel developers should reduce to a minimum the use of #define, and preferably use words in uppercase and/or with underscores, in any case not commonly used words.

Or maybe you should change your datastructure to not have member names the conflict with 7 year old well defined global symbols in the Linux kernel?

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

*** one of my data structures has a field named 'current'. ***

Quoted 4 lines · Daniele Lugli

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

*** one of my data structures has a field named 'current'. ***

gcc -Wshadow

Would it be a good idea to add -Wshadow to the kernel compile options by default ?

Quoted 5 lines · Chris Wedgwood

Quoted 2 lines · Daniele Lugli

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

Quoted 3 lines

Quoted 1 line

*** one of my data structures has a field named 'current'. ***

gcc -Wshadow

Would it be a good idea to add -Wshadow to the kernel compile options by default ?

Quoted 5 lines · Chris Wedgwood

Quoted 2 lines · Daniele Lugli

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

Quoted 3 lines

Quoted 1 line

*** one of my data structures has a field named 'current'. ***

gcc -Wshadow

Would it be a good idea to add -Wshadow to the kernel compile options by default ?

Quoted 6 lines · Rik van Riel

Quoted 5 lines · Chris Wedgwood

Quoted 2 lines · Daniele Lugli

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

*** one of my data structures has a field named 'current'. ***

gcc -Wshadow

Would it be a good idea to add -Wshadow to the kernel compile options by default ?

While I'm not defending macro abuse, please note that Daniele's problem appears to have been caused by using g++ instead of gcc or gcc -x c to compile a kernel module. Daniele's later example throws a syntax error in gcc, since the cpp output isn't legal C ...

Hence I fail to see the utility of hacking in kludges for something that's not supposed to work anyway.

But let me at least summarize my poor-programmer-not-kernel-developer point of view: at present the kernel if a mined field for c++ and i understand it is not viable nor interesting for the majority to rewrite it in a more c++-friendly way. But why not at least keep in mind, while writing new stuff (not the case of current.h i see), that kernel headers could be included by c++?

Quoted 15 lines · Mikael Pettersson

Quoted 6 lines · Rik van Riel

Quoted 5 lines · Chris Wedgwood

Quoted 2 lines · Daniele Lugli

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

*** one of my data structures has a field named 'current'. ***

gcc -Wshadow

Would it be a good idea to add -Wshadow to the kernel compile options by default ?

Quoted 7 lines

While I'm not defending macro abuse, please note that Daniele's problem appears to have been caused by using g++ instead of gcc or gcc -x c to compile a kernel module. Daniele's later example throws a syntax error in gcc, since the cpp output isn't legal C ...

Hence I fail to see the utility of hacking in kludges for something that's not supposed to work anyway.

Yes i confess, i'm writing a kernel module in c++ (and i'm not the only one).
Anyway my consideration was general and IMHO applies to C too. What is the benefit of redefining commonly used words? I would say nothing against eg #define _I386_current get_current(), but just #define current get_current() seems to me a little bit dangerous. What is the limit? What do you consider a bad practice? Would #define i j be tolerated?

But let me at least summarize my poor-programmer-not-kernel-developer point of view: at present the kernel if a mined field for c++ and i understand it is not viable nor interesting for the majority to rewrite it in a more c++-friendly way. But why not at least keep in mind, while writing new stuff (not the case of current.h i see), that kernel headers could be included by c++?

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

*** one of my data structures has a field named 'current'. ***

Pretty common word, isn't it? Would you think it can cause such a trouble? But in some of my files I happen to indirectly include <asm/current.h> (kernel 2.4.18 for i386), containing the following line:

#define current get_current()

so that my structure becomes the owner of a function it has never asked for, while it looses a data member. gcc has nothing to complain about that.

Quoted 1 line

#define current get_current()

This cannot be the reason for your problem. The name of a structure member has no connection whatsoever with the name of any function or definition.

Quoted 5 lines · Richard B. Johnson

Quoted 1 line

#define current get_current()

This cannot be the reason for your problem. The name of a structure member has no connection whatsoever with the name of any function or definition.

You forgot the effect of the parenthesis. This isn't a name-changing macro, this is a variable-to-function-call changing macro.

OG.

Quoted 9 lines · Daniele Lugli

I recently wrote a kernel module which gave me some mysterious problems. After too many days spent in blood, sweat and tears, I found the cause:

*** one of my data structures has a field named 'current'. ***

Pretty common word, isn't it? Would you think it can cause such a trouble? But in some of my files I happen to indirectly include <asm/current.h> (kernel 2.4.18 for i386), containing the following line:

Quoted 6 lines

#define current get_current()

so that my structure becomes the owner of a function it has never asked for, while it looses a data member. gcc has nothing to complain about that.

This cannot be the reason for your problem. The name of a structure member has no connection whatsoever with the name of any function or definition.

The following code will correctly write "Hello world!" to the screen even though the text initializes a member of a structure called "current" while "current" has been defined to be a function called puts.

#include <stdio.h>
#define current puts

struct foo {
    char *current;
    int foo;
    } bar;
main()
{
    bar.current = "Hello world!";
    current(bar.current);
    return 0;
}

For your code to get "confused", you really have something else wrong. That said, some name-space polution may make it difficult to find the problem. For instance, a structure member is expected to have a ";" after it. It's possible for some previous definition to make a syntax error invisible.

Cheers,
Dick Johnson

Try the following instead:

// file shade1.cpp

// excerpt from common.h

#define current get_current()

// my stuff

struct {
int any;
int current[1000];
} mine;

// file shade2.cpp

#include <stdio.h>

// my stuff

extern struct {
int any;
int current[1000];
} mine;

int main () {
mine.current[999] = 1;
printf ("%d\n", mine.current[999]);
}

g++ shade1.cpp shade2.cpp

./a.out => segmentation fault

Regards, Daniele

*** one of my data structures has a field named 'current'. ***

Pretty common word, isn't it? Would you think it can cause such a trouble? But in some of my files I happen to indirectly include <asm/current.h> (kernel 2.4.18 for i386), containing the following line:

#define current get_current()

How about changing the definition to:

#define current ((struct task_struct *)get_current())