On Mon, 4 May 2015, Miod Vallat wrote:
> > > Index: libc/stdlib/posix_pty.c
> > > ===================================================================
> > > RCS file: /cvs/src/lib/libc/stdlib/posix_pty.c,v
> > > retrieving revision 1.1
> > > diff -u -p -r1.1 posix_pty.c
> > > --- libc/stdlib/posix_pty.c 3 Dec 2012 20:08:33 -0000 1.1
> > > +++ libc/stdlib/posix_pty.c 4 May 2015 04:48:11 -0000
> > > @@ -35,13 +35,14 @@ posix_openpt(int oflag)
> > > int fd, mfd = -1;
> > >
> > > /* User must specify O_RDWR in oflag. */
> > > - if (!(oflag & O_RDWR)) {
> > > + if ((oflag & O_ACCMODE) != O_RDWR ||
> > > + (oflag & ~(O_ACCMODE | O_NOCTTY)) != 0) {
> > > errno = EINVAL;
> > > return -1;
> > > }
> >
> > I don't see the need to use O_ACCMODE here at all. Why not just:
> >
> > if (!(oflag & O_RDWR) || (oflag & ~(O_RDWR | O_NOCTTY)) != 0)
>
> because this is as ugly as the existing code, but harder to understand.
It's also wrong if O_RDWR has multiple bits set and there's another mode
with a subset of those set...as O_SEARCH or O_EXEC may well be on some
systems.
> > > Index: libkvm/kvm.c
>
> > > - if (flag & ~O_ACCMODE) {
> > > + if ((flag & ~O_ACCMODE) || (flag != O_RDONLY && flag != O_WRONLY &&
> > > + flag != O_RDWR)){
> >
> > The check against O_ACCMODE seems superfluous here since you added
> > the explicit flag value checks.
>
> Agreed.
Point. I'll drop that.
Philip
