Re: ideas

Brian M Grunkemeyer (bg2k+@andrew.cmu.edu)
Thu, 9 May 1996 15:31:32 -0400 (EDT)


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