* [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list
@ 2007-01-10 16:34 jerome Arbez-Gindre
2007-01-11 13:56 ` jerome Arbez-Gindre
0 siblings, 1 reply; 4+ messages in thread
From: jerome Arbez-Gindre @ 2007-01-10 16:34 UTC (permalink / raw)
To: qemu-devel
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.
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 10 Jan 2007 16:06:45 -0000
@@ -5926,6 +5926,11 @@
if (FD_ISSET(ioh->fd, &rfds)) {
ioh->fd_read(ioh->opaque);
}
+ }
+ /* the IOHandlerRecord could have been removed from the list
+ and freed during ioh->fd_read call */
+ for(ioh = first_io_handler; ioh != NULL; ioh = ioh_next) {
+ ioh_next = ioh->next;
if (FD_ISSET(ioh->fd, &wfds)) {
ioh->fd_write(ioh->opaque);
}
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list
2007-01-10 16:34 [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list jerome Arbez-Gindre
@ 2007-01-11 13:56 ` jerome Arbez-Gindre
2007-01-11 15:00 ` jerome Arbez-Gindre
0 siblings, 1 reply; 4+ messages in thread
From: jerome Arbez-Gindre @ 2007-01-11 13:56 UTC (permalink / raw)
To: qemu-devel
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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list
2007-01-11 13:56 ` jerome Arbez-Gindre
@ 2007-01-11 15:00 ` jerome Arbez-Gindre
2007-01-11 19:58 ` Fabrice Bellard
0 siblings, 1 reply; 4+ messages in thread
From: jerome Arbez-Gindre @ 2007-01-11 15:00 UTC (permalink / raw)
To: qemu-devel
On Thu, 2007-01-11 at 14:56 +0100, jerome Arbez-Gindre wrote:
> 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.
Here is a little fix to handle the case when a IOHandler removes an
other IOHandler.
--- vl.c.mine 2007-01-11 15:06:47.000000000 +0100
+++ vl.c 2007-01-11 15:27:27.000000000 +0100
@@ -5912,11 +5912,13 @@
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 could have been set to null by an other
+ IOHandlerRecord callback */
+ if (ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
ioh->fd_read(ioh->opaque);
}
/* ioh->fd_write could have been set to null */
- if ((ioh->fd_write) && (FD_ISSET(ioh->fd, &wfds))) {
+ if (ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
ioh->fd_write(ioh->opaque);
}
/* the ioh could have been supressed */
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list
2007-01-11 15:00 ` jerome Arbez-Gindre
@ 2007-01-11 19:58 ` Fabrice Bellard
0 siblings, 0 replies; 4+ messages in thread
From: Fabrice Bellard @ 2007-01-11 19:58 UTC (permalink / raw)
To: qemu-devel
I think a more complete patch is needed to fully correct the problem.
Regards,
Fabrice.
jerome Arbez-Gindre wrote:
> On Thu, 2007-01-11 at 14:56 +0100, jerome Arbez-Gindre wrote:
>
>>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.
>
>
> Here is a little fix to handle the case when a IOHandler removes an
> other IOHandler.
>
> --- vl.c.mine 2007-01-11 15:06:47.000000000 +0100
> +++ vl.c 2007-01-11 15:27:27.000000000 +0100
> @@ -5912,11 +5912,13 @@
> 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 could have been set to null by an other
> + IOHandlerRecord callback */
> + if (ioh->fd_read && FD_ISSET(ioh->fd, &rfds)) {
> ioh->fd_read(ioh->opaque);
> }
> /* ioh->fd_write could have been set to null */
> - if ((ioh->fd_write) && (FD_ISSET(ioh->fd, &wfds))) {
> + if (ioh->fd_write && FD_ISSET(ioh->fd, &wfds)) {
> ioh->fd_write(ioh->opaque);
> }
> /* the ioh could have been supressed */
>
>
>
>
> _______________________________________________
> Qemu-devel mailing list
> Qemu-devel@nongnu.org
> http://lists.nongnu.org/mailman/listinfo/qemu-devel
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-01-11 20:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-01-10 16:34 [Qemu-devel] [PATCH] better handling of removal in IOHandlerRecord list jerome Arbez-Gindre
2007-01-11 13:56 ` jerome Arbez-Gindre
2007-01-11 15:00 ` jerome Arbez-Gindre
2007-01-11 19:58 ` Fabrice Bellard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).