Re: Possible SERIOUS bug in open()?

Tim Newsham (newsham@ALOHA.NET)
Sat, 25 Oct 1997 09:26:12 -1000

> > + if(!flags)
> > + flags++;
> > +
>
> This will only cover the -1 case. Perhaps also changing:
>
> > flags = FFLAGS(uap->flags);
>
> to
>
> > flags = FFLAGS(uap->flags) & 3;
>
> and then the zero test as above?

What about the higher flag bits? (O_APPEND, O_CREAT, ...)

What FFLAGS is supposed to do is remap the low two bits:

O_RDONLY 0 -> FREAD 1
O_WRONLY 1 -> FWRITE 2
O_RDWR 2 -> FREAD | FWRITE 3

this would be much more clear and much less error-prone if
this was done explicitely instead of with a clever hack that
obfuscates the operation. Implementing it in this way
makes it clear what to do:

switch(uap->flags & O_ACCMODE) {
case O_RDONLY:
lowbits = FREAD;
break;
case O_WRONLY:
lowbits = FWRITE;
break;
case O_RDWR:
lowbits = FREAD | FWRITE;
break;
default:
return EINVAL;
}
flags = (uap->flags & ~O_ACCMODE) | lowbits;

yes this will mean that open and F_SETFL will be slightly
slower, but it is probably not noticeable, and the extra
clarity is worth it.

> -mm-

Tim N.