when compiling 2.1.92 on i386 with gcc 2.8.1 and additional
warnings (-W), I got these reports. I am just mentioning
dubious cases where unsigned variables are checked for negative
values. GCC's warning for this is
warning: unsigned value < 0 is always 0
In mm/memory.c, function zap_page_range(), line 404:
/*
* Update rss for the mm_struct (not necessarily current->mm)
*/
if (mm->rss > 0) {
mm->rss -= freed;
if (mm->rss < 0)
^^^^^^^^^^^
mm->rss = 0;
}
If any underflows should be checked, this does not work!
'mm->rss -= freed;' never yields something negative since rss
is declared unsigned.
if (mm->rss >= freed)
mm->rss -= freed;
else
mm->rss = 0;
is suggested instead.
--- mm/memory.c.or Sat Mar 14 13:37:17 1998
+++ mm/memory.c Sun Apr 5 18:31:39 1998
@@ -399,10 +399,10 @@
/*
* Update rss for the mm_struct (not necessarily current->mm)
*/
- if (mm->rss > 0) {
+ if (mm->rss >= freed) {
mm->rss -= freed;
- if (mm->rss < 0)
- mm->rss = 0;
+ } else {
+ mm->rss = 0;
}
}
---------
drivers/char/mem.c function do_write_mem(), line 63 and
function read_mem(), line 106:
if (copy_from_user(p, buf, count) < 0)
return -EFAULT;
Since the declarations for copy_from_user() are:
static inline unsigned long __constant_copy_from_user(...)
unsigned long __generic_copy_from_user(...);
the error return branch is probably optimized away, since it
cannot happen.
---------
Similar in drivers/char/random.c line 1220
if (random_state.entropy_count < 0)
random_state.entropy_count = 0;
and in line 1279
These two lines can be omitted, unless entropy_count is planned
to become signed sometime.
---------
ditto in net/socket.c, line 946
if (len < 0)
goto out_put;
can never happen.
---------
Similar it would be nice, if the loop in macro _SIG_SET_BINOP in
include/linux/signal.h could be changed to avoid warnings here.
--- include/linux/signal.h.or Sun Apr 5 15:36:06 1998
+++ include/linux/signal.h Sun Apr 5 17:23:06 1998
@@ -69,7 +69,7 @@
unsigned long a0, a1, a2, a3, b0, b1, b2, b3; \
unsigned long i; \
\
- for (i = 0; i < _NSIG_WORDS/4; ++i) { \
+ for (i = 0; (long) i < _NSIG_WORDS/4; ++i) { \
a0 = a->sig[4*i+0]; a1 = a->sig[4*i+1]; \
a2 = a->sig[4*i+2]; a3 = a->sig[4*i+3]; \
b0 = b->sig[4*i+0]; b1 = b->sig[4*i+1]; \
@@ -119,7 +119,7 @@
{ \
unsigned long i; \
\
- for (i = 0; i < _NSIG_WORDS/4; ++i) { \
+ for (i = 0; (long) i < _NSIG_WORDS/4; ++i) { \
set->sig[4*i+0] = op(set->sig[4*i+0]); \
set->sig[4*i+1] = op(set->sig[4*i+1]); \
set->sig[4*i+2] = op(set->sig[4*i+2]); \
---------
Other minor warnings:
In drivers/char/serial.c:
serial.c:2781: warning: `inline' is not at beginning of declaration
In net/sched
sch_generic.c: In function `pfifo_fast_enqueue':
sch_generic.c:132: warning: `static' is not at beginning of declaration
sch_generic.c: In function `pfifo_fast_requeue':
sch_generic.c:164: warning: `static' is not at beginning of declaration
The rest of the warnings is mainly 'variable xxx not used' and tons of
comaprisons between signed and unsigned. I have been too lazy to check
all of the latter, but it might be worthwhile.
Heiko
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu