| Message ID | 20180301084438.13594-12-peterx@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qio: general non-default GMainContext support | expand |
On Thu, Mar 01, 2018 at 04:44:34PM +0800, Peter Xu wrote: > qio_channel_tls_handshake_full() is introduced to allow the TLS to be > run on a non-default context. Still, no functional change. > > Signed-off-by: Peter Xu <peterx@redhat.com> > --- > include/io/channel-tls.h | 17 ++++++++++++++++ > io/channel-tls.c | 51 +++++++++++++++++++++++++++++++++++------------- > 2 files changed, 54 insertions(+), 14 deletions(-) > > static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, > - QIOTask *task) > + QIOTask *task, > + GMainContext *context) > { > Error *err = NULL; > QCryptoTLSSessionHandshakeStatus status; > @@ -171,6 +177,11 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, > qio_task_complete(task); > } else { > GIOCondition condition; > + QIOChannelTLSData *data = g_new0(typeof(*data), 1); > + > + data->task = task; > + data->context = context; The 'context' reference is only valid for as long as the caller exists. So you need to acquire a reference on 'context' here.... > @@ -191,20 +203,23 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc, > GIOCondition condition, > gpointer user_data) > { > - QIOTask *task = user_data; > + QIOChannelTLSData *data = user_data; > + QIOTask *task = data->task; > + GMainContext *context = data->context; > QIOChannelTLS *tioc = QIO_CHANNEL_TLS( > qio_task_get_source(task)); > > - qio_channel_tls_handshake_task( > - tioc, task); > + g_free(data); > + qio_channel_tls_handshake_task(tioc, task, context); And release the reference on context here. Regards, Daniel
On 01/03/2018 09:44, Peter Xu wrote: > +/** > + * qio_channel_tls_handshake_full: > + * @ioc: the TLS channel object > + * @func: the callback to invoke when completed > + * @opaque: opaque data to pass to @func > + * @destroy: optional callback to free @opaque > + * @context: the context that TLS handshake will run with > + * > + * Similar to qio_channel_tls_handshake(), but allows the task to be > + * run on a specific context. > + */ > +void qio_channel_tls_handshake_full(QIOChannelTLS *ioc, > + QIOTaskFunc func, > + gpointer opaque, > + GDestroyNotify destroy, > + GMainContext *context); > + You're not consistent in introducing "_full" functions. I would add the argument directly to the qio_channel_tls_handshake() function. Paolo
On Thu, Mar 01, 2018 at 06:22:40PM +0100, Paolo Bonzini wrote: > On 01/03/2018 09:44, Peter Xu wrote: > > +/** > > + * qio_channel_tls_handshake_full: > > + * @ioc: the TLS channel object > > + * @func: the callback to invoke when completed > > + * @opaque: opaque data to pass to @func > > + * @destroy: optional callback to free @opaque > > + * @context: the context that TLS handshake will run with > > + * > > + * Similar to qio_channel_tls_handshake(), but allows the task to be > > + * run on a specific context. > > + */ > > +void qio_channel_tls_handshake_full(QIOChannelTLS *ioc, > > + QIOTaskFunc func, > > + gpointer opaque, > > + GDestroyNotify destroy, > > + GMainContext *context); > > + > > You're not consistent in introducing "_full" functions. I would > add the argument directly to the qio_channel_tls_handshake() function. Will take your advise. Thanks,
On Thu, Mar 01, 2018 at 03:50:01PM +0000, Daniel P. Berrangé wrote: > On Thu, Mar 01, 2018 at 04:44:34PM +0800, Peter Xu wrote: > > qio_channel_tls_handshake_full() is introduced to allow the TLS to be > > run on a non-default context. Still, no functional change. > > > > Signed-off-by: Peter Xu <peterx@redhat.com> > > --- > > include/io/channel-tls.h | 17 ++++++++++++++++ > > io/channel-tls.c | 51 +++++++++++++++++++++++++++++++++++------------- > > 2 files changed, 54 insertions(+), 14 deletions(-) > > > > > static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, > > - QIOTask *task) > > + QIOTask *task, > > + GMainContext *context) > > { > > Error *err = NULL; > > QCryptoTLSSessionHandshakeStatus status; > > @@ -171,6 +177,11 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, > > qio_task_complete(task); > > } else { > > GIOCondition condition; > > + QIOChannelTLSData *data = g_new0(typeof(*data), 1); > > + > > + data->task = task; > > + data->context = context; > > The 'context' reference is only valid for as long as the caller > exists. So you need to acquire a reference on 'context' here.... > > > > @@ -191,20 +203,23 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc, > > GIOCondition condition, > > gpointer user_data) > > { > > - QIOTask *task = user_data; > > + QIOChannelTLSData *data = user_data; > > + QIOTask *task = data->task; > > + GMainContext *context = data->context; > > QIOChannelTLS *tioc = QIO_CHANNEL_TLS( > > qio_task_get_source(task)); > > > > - qio_channel_tls_handshake_task( > > - tioc, task); > > > + g_free(data); > > + qio_channel_tls_handshake_task(tioc, task, context); > > And release the reference on context here. Yeah, fixed both. Thanks,
diff --git a/include/io/channel-tls.h b/include/io/channel-tls.h index d157eb10e8..d6f5271088 100644 --- a/include/io/channel-tls.h +++ b/include/io/channel-tls.h @@ -128,6 +128,23 @@ void qio_channel_tls_handshake(QIOChannelTLS *ioc, gpointer opaque, GDestroyNotify destroy); +/** + * qio_channel_tls_handshake_full: + * @ioc: the TLS channel object + * @func: the callback to invoke when completed + * @opaque: opaque data to pass to @func + * @destroy: optional callback to free @opaque + * @context: the context that TLS handshake will run with + * + * Similar to qio_channel_tls_handshake(), but allows the task to be + * run on a specific context. + */ +void qio_channel_tls_handshake_full(QIOChannelTLS *ioc, + QIOTaskFunc func, + gpointer opaque, + GDestroyNotify destroy, + GMainContext *context); + /** * qio_channel_tls_get_session: * @ioc: the TLS channel object diff --git a/io/channel-tls.c b/io/channel-tls.c index 6182702dab..4fe03d9c6c 100644 --- a/io/channel-tls.c +++ b/io/channel-tls.c @@ -140,13 +140,19 @@ qio_channel_tls_new_client(QIOChannel *master, return NULL; } +struct QIOChannelTLSData { + QIOTask *task; + GMainContext *context; +}; +typedef struct QIOChannelTLSData QIOChannelTLSData; static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc, GIOCondition condition, gpointer user_data); static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, - QIOTask *task) + QIOTask *task, + GMainContext *context) { Error *err = NULL; QCryptoTLSSessionHandshakeStatus status; @@ -171,6 +177,11 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, qio_task_complete(task); } else { GIOCondition condition; + QIOChannelTLSData *data = g_new0(typeof(*data), 1); + + data->task = task; + data->context = context; + if (status == QCRYPTO_TLS_HANDSHAKE_SENDING) { condition = G_IO_OUT; } else { @@ -178,11 +189,12 @@ static void qio_channel_tls_handshake_task(QIOChannelTLS *ioc, } trace_qio_channel_tls_handshake_pending(ioc, status); - qio_channel_add_watch(ioc->master, - condition, - qio_channel_tls_handshake_io, - task, - NULL); + qio_channel_add_watch_full(ioc->master, + condition, + qio_channel_tls_handshake_io, + data, + NULL, + context); } } @@ -191,20 +203,23 @@ static gboolean qio_channel_tls_handshake_io(QIOChannel *ioc, GIOCondition condition, gpointer user_data) { - QIOTask *task = user_data; + QIOChannelTLSData *data = user_data; + QIOTask *task = data->task; + GMainContext *context = data->context; QIOChannelTLS *tioc = QIO_CHANNEL_TLS( qio_task_get_source(task)); - qio_channel_tls_handshake_task( - tioc, task); + g_free(data); + qio_channel_tls_handshake_task(tioc, task, context); return FALSE; } -void qio_channel_tls_handshake(QIOChannelTLS *ioc, - QIOTaskFunc func, - gpointer opaque, - GDestroyNotify destroy) +void qio_channel_tls_handshake_full(QIOChannelTLS *ioc, + QIOTaskFunc func, + gpointer opaque, + GDestroyNotify destroy, + GMainContext *context) { QIOTask *task; @@ -212,7 +227,15 @@ void qio_channel_tls_handshake(QIOChannelTLS *ioc, func, opaque, destroy); trace_qio_channel_tls_handshake_start(ioc); - qio_channel_tls_handshake_task(ioc, task); + qio_channel_tls_handshake_task(ioc, task, context); +} + +void qio_channel_tls_handshake(QIOChannelTLS *ioc, + QIOTaskFunc func, + gpointer opaque, + GDestroyNotify destroy) +{ + qio_channel_tls_handshake_full(ioc, func, opaque, destroy, NULL); }
qio_channel_tls_handshake_full() is introduced to allow the TLS to be run on a non-default context. Still, no functional change. Signed-off-by: Peter Xu <peterx@redhat.com> --- include/io/channel-tls.h | 17 ++++++++++++++++ io/channel-tls.c | 51 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 54 insertions(+), 14 deletions(-)