Re: Possible SERIOUS bug in open()?

[newsham () ALOHA NET (Tim Newsham)]

> > +       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.