From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1H50QF-0005Kw-Un for qemu-devel@nongnu.org; Thu, 11 Jan 2007 08:56:52 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1H50QD-0005KM-Tm for qemu-devel@nongnu.org; Thu, 11 Jan 2007 08:56:51 -0500 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1H50QD-0005KH-QC for qemu-devel@nongnu.org; Thu, 11 Jan 2007 08:56:49 -0500 Received: from [81.255.54.11] (helo=mx.laposte.net) by monty-python.gnu.org with esmtp (Exim 4.52) id 1H50QD-0003ZR-63 for qemu-devel@nongnu.org; Thu, 11 Jan 2007 08:56:49 -0500 Received: from smtp.laposte.net (10.150.9.38) by mx.laposte.net (7.2.060.1) id 45A22559002F74F8 for qemu-devel@nongnu.org; Thu, 11 Jan 2007 14:56:48 +0100 Received: from smtpin.laposte.net (10.150.9.73) by smtp.laposte.net (7.3.105.2) id 459FB4F8005BDFA3 for qemu-devel@nongnu.org; Thu, 11 Jan 2007 15:04:30 +0100 Received: from bibi (217.128.241.130) by smtpin.laposte.net (7.2.060.1) (authenticated as jerome.arbez-gindre) id 45864E240024C6B9 for qemu-devel@nongnu.org; Thu, 11 Jan 2007 14:56:05 +0100 Subject: Re: [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list From: jerome Arbez-Gindre In-Reply-To: <1168446890.24524.11.camel@bibi> References: <1168446890.24524.11.camel@bibi> Content-Type: text/plain Date: Thu, 11 Jan 2007 14:56:41 +0100 Message-Id: <1168523801.4765.10.camel@bibi> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On Wed, 2007-01-10 at 17:34 +0100, jerome Arbez-Gindre wrote: > Hi, > > by a call to qemu_set_fd_handler(fd,NULL,NULL,NULL) in the fd_read > callback, I have generated a "Segmentation fault" in vl.c. > > My solution is not very smart... but it is very simple. I reply to myself because I did not sleep last night: Here is the fix without the double IOHandlerRecord list iteration. Index: vl.c =================================================================== RCS file: /sources/qemu/qemu/vl.c,v retrieving revision 1.236 diff -u -r1.236 vl.c --- vl.c 9 Jan 2007 19:44:41 -0000 1.236 +++ vl.c 11 Jan 2007 13:55:52 -0000 @@ -4179,38 +4179,26 @@ IOHandler *fd_write, void *opaque) { - IOHandlerRecord **pioh, *ioh; + IOHandlerRecord *ioh; - if (!fd_read && !fd_write) { - pioh = &first_io_handler; - for(;;) { - ioh = *pioh; - if (ioh == NULL) - break; - if (ioh->fd == fd) { - *pioh = ioh->next; - qemu_free(ioh); - break; - } - pioh = &ioh->next; - } - } else { - for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { - if (ioh->fd == fd) - goto found; - } - ioh = qemu_mallocz(sizeof(IOHandlerRecord)); - if (!ioh) - return -1; - ioh->next = first_io_handler; - first_io_handler = ioh; - found: - ioh->fd = fd; - ioh->fd_read_poll = fd_read_poll; - ioh->fd_read = fd_read; - ioh->fd_write = fd_write; - ioh->opaque = opaque; + for(ioh = first_io_handler; ioh != NULL; ioh = ioh->next) { + if (ioh->fd == fd) + goto found; } + if (!fd_read && !fd_write) + return 0 ; + ioh = qemu_mallocz(sizeof(IOHandlerRecord)); + if (!ioh) + return -1; + ioh->next = first_io_handler; + first_io_handler = ioh; +found: + ioh->fd = fd; + ioh->fd_read_poll = fd_read_poll; + ioh->fd_read = fd_read; + ioh->fd_write = fd_write; + ioh->opaque = opaque; + return 0; } @@ -5858,7 +5846,7 @@ void main_loop_wait(int timeout) { - IOHandlerRecord *ioh, *ioh_next; + IOHandlerRecord **pioh, *ioh, *ioh_next; fd_set rfds, wfds, xfds; int ret, nfds; struct timeval tv; @@ -5921,14 +5909,23 @@ ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv); if (ret > 0) { /* XXX: better handling of removal */ + pioh = &first_io_handler ; for(ioh = first_io_handler; ioh != NULL; ioh = ioh_next) { ioh_next = ioh->next; if (FD_ISSET(ioh->fd, &rfds)) { ioh->fd_read(ioh->opaque); } - if (FD_ISSET(ioh->fd, &wfds)) { + /* ioh->fd_write could have been set to null */ + if ((ioh->fd_write) && (FD_ISSET(ioh->fd, &wfds))) { ioh->fd_write(ioh->opaque); } + /* the ioh could have been supressed */ + if (!ioh->fd_write && !ioh->fd_read) { + *pioh = ioh_next; + qemu_free(ioh); + } else { + pioh = &ioh->next ; + } } } #if defined(CONFIG_SLIRP)