This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/6] Move notif_queue to remote_state
- From: Pedro Alves <palves at redhat dot com>
- To: Yao Qi <yao at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 02 Oct 2013 11:48:26 +0100
- Subject: Re: [PATCH 1/6] Move notif_queue to remote_state
- Authentication-results: sourceware.org; auth=none
- References: <1376877311-4135-1-git-send-email-yao at codesourcery dot com> <1376877311-4135-2-git-send-email-yao at codesourcery dot com> <52430B69 dot 4090301 at redhat dot com> <5249293D dot 8050700 at codesourcery dot com> <5249B011 dot 1080207 at redhat dot com> <524B7C65 dot 3030005 at codesourcery dot com>
On 10/02/2013 02:52 AM, Yao Qi wrote:
> On 10/01/2013 01:08 AM, Pedro Alves wrote:
>>> -static void
>>>> -notif_xfree (struct notif_client *notif)
>> ...
>>>> - if (notif->pending_event != NULL
>>>> - && notif->pending_event->dtr != NULL)
>>>> - notif->pending_event->dtr (notif->pending_event);
>> Hmm, it's not clear to me from reading the patch where is
>> pending_event released after this patch. Could you help
>> me understand that?
>
> Since notif_queue is a global variable, we've never used QUEUE_free to
> free notif_queue, so notif_xfree is never called, and pending_event on
> each notif_client is not released.
>
> When we move notif_queue to 'remote_notif_state', we have to explicitly
> use QUEUE_free to free notif_queue and elements in it. We put the
> pointer of each notif_client to notif_queue, so we don't need a function
> to free the notif_client when notif_queue is destroyed. So the
> notif_queue is allocated like this,
>
> notif_state->notif_queue = QUEUE_alloc (notif_client_p, NULL);
>
> and notif_xfree is not needed any more. The leak on pending_event is
> still there, but it is fixed by the next patch, which moves
> pending_event to remote_notif_state, and pending_event is freed in
> remote_notif_state_xfree.
I see now, thanks.
BTW:
+struct remote_notif_state
+{
+ /* Notification queue. */
+ QUEUE(notif_client_p) *notif_queue;
+
+ /* Asynchronous signal handle registered as event loop source for when
+ the remote sent us a notification. The registered callback
+ will do a ACK sequence to pull the rest of the events out of
+ the remote side into our event queue. */
+
+ struct async_event_handler *remote_async_get_pending_events_token;
Since remote_async_get_pending_events_token is no longer a global,
it doesn't need the "remote_" prefix (or even remote_async_?) anymore.
WDYT?
The patch is OK with or without that change.
Thanks,
--
Pedro Alves