On Wed, Nov 05, 2014 at 02:44:54PM -0800, patrick keshishian wrote:
> On Wed, Nov 05, 2014 at 08:45:07PM +0100, Theo Buehler wrote:
> > Pausing a tetris game currently causes a segfault due to a a null
> > pointer dereference.
> >
> > Fix this by checking that s is non-NULL before accessing its members.
> >
> > A number of comments and an error message still refer to select()
> > instead of poll(). Correct this as well.
> >
> >
> > Index: input.c
> > ===================================================================
> > RCS file: /cvs/src/games/tetris/input.c,v
> > retrieving revision 1.13
> > diff -u -p -r1.13 input.c
> > --- input.c 3 Nov 2014 22:14:54 -0000 1.13
> > +++ input.c 5 Nov 2014 19:39:30 -0000
> > @@ -64,12 +64,12 @@
> > }
> >
> > /*
> > - * Do a `read wait': select for reading from stdin, with timeout *tvp.
> > + * Do a `read wait': poll for reading from stdin, with timeout *tvp.
> > * On return, modify *tvp to reflect the amount of time spent waiting.
> > * It will be positive only if input appeared before the time ran out;
> > * otherwise it will be zero or perhaps negative.
> > *
> > - * If tvp is nil, wait forever, but return if select is interrupted.
> > + * If tvp is nil, wait forever, but return if poll is interrupted.
> > *
> > * Return 0 => no input, 1 => can read() from stdin
> > */
> > @@ -90,14 +90,15 @@ rwait(struct timeval *tvp)
> > again:
> > pfd[0].fd = STDIN_FILENO;
> > pfd[0].events = POLLIN;
> > - switch (poll(pfd, 1, s->tv_sec * 1000 + s->tv_usec / 1000)) {
> > + switch (poll(pfd, 1, s ? s->tv_sec * 1000 + s->tv_usec / 1000 :
>
>
> I propose getting rid of the s pointer all together as such:
After replacing select() with poll(), not removing
`struct timeval *s' seems an oversight; Its use was
solely for select()'s benefit.
Once more, proposing removal of `struct timeval *s' and
using an `int timo' for the time-out value, that gets fed
into poll(). It also improves readability of the code;
rumors floating around that that is a good thing.
Index: input.c
===================================================================
RCS file: /cvs/obsd/src/games/tetris/input.c,v
retrieving revision 1.14
diff -u -p -u -r1.14 input.c
--- input.c 5 Nov 2014 20:23:38 -0000 1.14
+++ input.c 7 Nov 2014 19:37:07 -0000
@@ -76,7 +76,8 @@
int
rwait(struct timeval *tvp)
{
- struct timeval starttv, endtv, *s;
+ int timo = INFTIM;
+ struct timeval starttv, endtv;
struct pollfd pfd[1];
#define NILTZ ((struct timezone *)0)
@@ -84,15 +85,12 @@ rwait(struct timeval *tvp)
if (tvp) {
(void) gettimeofday(&starttv, NILTZ);
endtv = *tvp;
- s = &endtv;
- } else
- s = NULL;
+ timo = endtv.tv_sec * 1000 + endtv.tv_usec / 1000;
+ }
again:
pfd[0].fd = STDIN_FILENO;
pfd[0].events = POLLIN;
- switch (poll(pfd, 1, s ? s->tv_sec * 1000 + s->tv_usec / 1000 :
- INFTIM)) {
-
+ switch (poll(pfd, 1, timo)) {
case -1:
if (tvp == 0)
return (-1);
>
> Index: input.c
> ===================================================================
> RCS file: /cvs/obsd/src/games/tetris/input.c,v
> retrieving revision 1.13
> diff -u -p -u -p -r1.13 input.c
> --- input.c 3 Nov 2014 22:14:54 -0000 1.13
> +++ input.c 5 Nov 2014 22:40:47 -0000
> @@ -64,19 +64,20 @@
> }
>
> /*
> - * Do a `read wait': select for reading from stdin, with timeout *tvp.
> + * Do a `read wait': poll for reading from stdin, with timeout *tvp.
> * On return, modify *tvp to reflect the amount of time spent waiting.
> * It will be positive only if input appeared before the time ran out;
> * otherwise it will be zero or perhaps negative.
> *
> - * If tvp is nil, wait forever, but return if select is interrupted.
> + * If tvp is nil, wait forever, but return if poll is interrupted.
> *
> * Return 0 => no input, 1 => can read() from stdin
> */
> int
> rwait(struct timeval *tvp)
> {
> - struct timeval starttv, endtv, *s;
> + int timo = INFTIM;
> + struct timeval starttv, endtv;
> struct pollfd pfd[1];
>
> #define NILTZ ((struct timezone *)0)
> @@ -84,20 +85,19 @@ rwait(struct timeval *tvp)
> if (tvp) {
> (void) gettimeofday(&starttv, NILTZ);
> endtv = *tvp;
> - s = &endtv;
> - } else
> - s = NULL;
> + timo = endtv.tv_sec * 1000 + endtv.tv_usec / 1000;
> + }
> again:
> pfd[0].fd = STDIN_FILENO;
> pfd[0].events = POLLIN;
> - switch (poll(pfd, 1, s->tv_sec * 1000 + s->tv_usec / 1000)) {
> + switch (poll(pfd, 1, timo)) {
>
> case -1:
> if (tvp == 0)
> return (-1);
> if (errno == EINTR)
> goto again;
> - stop("select failed, help");
> + stop("poll failed, help");
> /* NOTREACHED */
>
> case 0: /* timed out */
> @@ -115,7 +115,7 @@ again:
> }
>
> /*
> - * `sleep' for the current turn time (using select).
> + * `sleep' for the current turn time (using poll).
> * Eat any input that might be available.
> */
> void
>
>
>
> --patrick
>
>
>
>
>
>
>
>
>
> > + INFTIM)) {
> >
> > case -1:
> > if (tvp == 0)
> > return (-1);
> > if (errno == EINTR)
> > goto again;
> > - stop("select failed, help");
> > + stop("poll failed, help");
> > /* NOTREACHED */
> >
> > case 0: /* timed out */
> > @@ -115,7 +116,7 @@ again:
> > }
> >
> > /*
> > - * `sleep' for the current turn time (using select).
> > + * `sleep' for the current turn time (using poll).
> > * Eat any input that might be available.
> > */
> > void
> >
>
