Messages in this thread | ![/](/images/icornerl.gif) | | Date | Thu, 9 May 1996 15:31:32 -0400 (EDT) | From | Brian M Grunkemeyer <> | Subject | Re: ideas |
| |
Excerpts from mail: 9-May-96 Re: ideas by "J. Sean Connell"@connor > > g++ caught several additional types of (small?) errors: not handling > > values falling through switch statements, > Could you please clarify?
Ok, once again I'll append the output of a compile w/ g++ of just the first file (it doesn't get past init/main.c) to the end of this message. This specific problem can be shown like this:
/* very contrived example */ int boolean_case(int n) { switch (n) { case 0: /* do stuff to make n true */ return 0; break; case 1: return 1; break; } }
C++ would complain because n isn't guaranteed to be either 0 or 1. If n is not in that expected range, then the function doesn't return anything. The author of the function clearly expected the value to be 0 or 1, but what if the function was accidentally misused, being passed a value like -1 or 2. Or if memory got corrupted (which could NEVER happen in the linux kernel ]:> ), any value could get into this function. What then? It should at the very least return some half-way reasonable value, if not print an error message, when passed an unexpected input. G++ says warning: control reached end of non-void function "function_name".
> > comparisons between signed and unsigned ints, > g++ w/o -Wall won't catch this. (Never mind the fact that gcc won't > catch it at all.)
Ok, you're right on this one. I didn't notice the -Wall the kernel Makefile tacks on to the compile line somewhere.
> > and "illegal" conversion from int (*)(void*) to int(*)(char*). > > While none of these are really > > critical and C probably handles them all for us, it wouldn't hurt to > > make the types more clear and attempt to handle illegal inputs to > > procedures. Makes reading the code and tracking down bugs easier. > > > > My main objection is that the kernel is written in C, not C++, and no > matter how similar the languages may be, C is *not* C++, despite being > very similar. The items above that do not generate warnings, IMHO, do > not do so because there's nothing wrong with those constructs. (Mind > you, I think it'd be nice if gcc could be made to warn about unused > parameters.)
It's pretty well-known that C++ isn't a perfect superset of C. But those things that C++ does complain about often look like things we should probably fix, if nothing else for readability of the code. In the output below, there are two lines saying "ANSI C++ forbids implicit conversion ...", a function was taking a void* when it clearly meant to take a char *.
> On the other hand, we could go whole hog, and run it through that program > that was discussed here recently, or maybe just plain lint.
I've never used lint, but if it would catch most of these bugs (the "real" bugs, that is), then that would probably be a better tool. Does anyone have any experience with it?
BTW, does anyone know why people decided the extern "C" should be put in front of several functions? asmlinkage is defined to be extern "C" only if you're using C++. I know roughly what it does, but why is it needed?
g++ -D__KERNEL__ -I/mnt/new/tmp/linux/include -Wall -Wstrict-prototypes -O2 -fomit-frame-pointer -fno-strength-reduce -pipe -m486 -DCPU=486 -c -o init/main.o init/main.c In file included from /mnt/new/tmp/linux/include/linux/sched.h:18, from init/main.c:19: /mnt/new/tmp/linux/include/linux/personality.h:26: parse error before string constant /mnt/new/tmp/linux/include/linux/personality.h:36: syntax error before `;' /mnt/new/tmp/linux/include/linux/personality.h:49: parse error before string constant /mnt/new/tmp/linux/include/asm/string.h: In function `void * __constant_memcpy(void *, const void *, unsigned int)': In file included from /mnt/new/tmp/linux/include/linux/string.h:38, from /mnt/new/tmp/linux/include/asm/termios.h:58, from /mnt/new/tmp/linux/include/linux/termios.h:5, from /mnt/new/tmp/linux/include/linux/tty.h:20, from /mnt/new/tmp/linux/include/linux/sched.h:25, from init/main.c:19: /mnt/new/tmp/linux/include/asm/string.h:443: warning: control reaches end of non-void function `__constant_memcpy(void *, const void *, unsigned int)' /mnt/new/tmp/linux/include/asm/string.h: In function `void * __constant_c_and_count_memset(void *, long unsigned int, unsigned int)': /mnt/new/tmp/linux/include/asm/string.h:592: warning: control reaches end of non-void function `__constant_c_and_count_memset(void *, long unsigned int, unsigned int)' /mnt/new/tmp/linux/include/linux/sched.h: In function `void select_wait(struct wait_queue **, struct select_table_struct *)': In file included from init/main.c:19: /mnt/new/tmp/linux/include/linux/sched.h:478: warning: comparison between signed and unsigned /mnt/new/tmp/linux/include/linux/mm.h: In function `int expand_stack(struct vm_area_struct *, long unsigned int)': In file included from init/main.c:32: /mnt/new/tmp/linux/include/linux/mm.h:321: warning: comparison between signed and unsigned /mnt/new/tmp/linux/include/asm/bugs.h: In function `void no_halt(char *, int *)': In file included from init/main.c:39: /mnt/new/tmp/linux/include/asm/bugs.h:19: warning: unused parameter `char * s' /mnt/new/tmp/linux/include/asm/bugs.h:19: warning: unused parameter `int * ints' /mnt/new/tmp/linux/include/asm/bugs.h: In function `void no_387(char *, int *)': /mnt/new/tmp/linux/include/asm/bugs.h:24: warning: unused parameter `char * s' /mnt/new/tmp/linux/include/asm/bugs.h:24: warning: unused parameter `int * ints' init/main.c: In function `void profile_setup(char *, int *)': init/main.c:221: warning: unused parameter `char * str' init/main.c: In function `void calibrate_delay()': init/main.c:465: warning: comparison between signed and unsigned init/main.c:482: warning: comparison between signed and unsigned init/main.c:485: warning: comparison between signed and unsigned init/main.c: In function `void parse_root_dev(char *)': init/main.c:502: warning: non-static const member `int const parse_root_dev(char *)::dev_name_struct::num' in class without a constructor init/main.c: In function `int cpu_idle(void *)': init/main.c:646: warning: unused parameter `void * unused' init/main.c: In function `int do_rc(void *)': init/main.c:843: warning: ANSI C++ forbids implicit conversion from `void *' in argument passing init/main.c: In function `int do_shell(void *)': init/main.c:855: warning: ANSI C++ forbids implicit conversion from `void *' in argument passing init/main.c: In function `int init(void *)': init/main.c:878: warning: unused parameter `void * unused' make: *** [init/main.o] Error 1
| ![\](/images/icornerr.gif) |