Thread (9 messages) 9 messages, 6 authors, 2004-11-23

Re: [PATCH] Remove pointless <0 comparison for unsigned variable in fs/fcntl.c

From: Jamie Lokier <hidden>
Date: 2004-11-23 10:42:15
Also in: lkml

Jesper Juhl wrote:
quoted
quoted
case F_SETSIG:
	/* arg == 0 restores default behaviour. */
-		if (arg < 0 || arg > _NSIG) {
+		if (arg > _NSIG) {
		break;
Let's find out.
The unusual thing about this function is that "arg" is really
polymorphic, but given type "unsigned long" in the kernel.  It is
really a way to hold arbitrary values of any type.

Just look at the way it becomes "unsigned int" (dupfd) or "struct
flock" (lock) or "long" (leases) or "int" (setown).

F_SETOWN is interesting because you really can pass a negative int
argument and get a meaningful result, even though it's passed around
as unsigned long for a little while.

Signal numbers are usually "int".  The intended behaviour of fcntl(fd,
F_SETSIG, sig) from userspace is that a negative sig returns EINVAL.

I.e. writing fcntl(fd, F_SETSIG, -1) in userspace will compile without
any warnings.  The intended behaviour is that a negative sig returns
EINVAL.  The kernel code illustrates that intention.

It isn't obvious that arg is unsigned long in this function, when
reading the code.  I had to scroll to the top of the function to check
that this patch doesn't change its behaviour.  For that reason I think
the "< 0" test is useful, as it illustrates the intended behaviour and
causes no harm.

-- Jamie
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help