On Mon, Nov 11, 2013 at 8:50 AM, Eric Blake <ebl...@redhat.com> wrote:
> Quick - identify the bug in this code (from ui/curses.c):
>
> static void curses_winch_handler(int signum)
> {
> struct winsize {
> unsigned short ws_row;
> unsigned short ws_col;
> unsigned short ws_xpixel; /* unused */
> unsigned short ws_ypixel; /* unused */
> } ws;
>
> /* terminal size changed */
> if (ioctl(1, TIOCGWINSZ, &ws) == -1)
> return;
>
> resize_term(ws.ws_row, ws.ws_col);
> curses_calc_pad();
> invalidate = 1;
>
> /* some systems require this */
> signal(SIGWINCH, curses_winch_handler);
> }
>
> Here's a hint: ioctl() can clobber errno. But if a signal handler is
> called in the middle of other code that is using errno, then the handler
> MUST restore the value of errno before returning, if it is to guarantee
> that the interrupted context won't be corrupted.
Isn't this precisely why EINTR exists?
Regards,
Anthony Liguori
>
> More reading on the topic:
> https://plus.google.com/+LennartPoetteringTheOneAndOnly/posts/gHSscCJkakd
>
> I have not done a full audit of qemu's signal handlers, so much as a
> quick look to see if I could find violations; it was surprisingly easy
> to find a bad example. A signal handler that resets the signal to
> SIG_DFL then calls raise() is exempt from caring about errno, but any
> signal handler that can fall through to the end and return execution to
> the caller MUST ensure that errno is left unchanged, for errno to be
> useful in the remaining body of code. Which is why the best signal
> handlers tend to be the one that only flag a volatile variable that is
> later checked at safe points of execution, rather than trying to make
> complex calls from within the handler context.
>
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
