From: Daniel Elstner Date: Fri, 11 Sep 2015 21:08:10 +0000 (+0200) Subject: serial: Make serial device event sources more robust X-Git-Tag: libsigrok-0.4.0~303 X-Git-Url: https://sigrok.org/gitweb/?p=libsigrok.git;a=commitdiff_plain;h=cbc1413f31f3946ce79da5540cfbeace8924c9d1 serial: Make serial device event sources more robust Disallow polling for input/error and output-ready events at the same time, and ensure only a single FD event source is installed. Also, do not leak if the FD event source is removed by means other than calling serial_source_remove(). --- diff --git a/src/libsigrok-internal.h b/src/libsigrok-internal.h index 465f2421..8fb9006a 100644 --- a/src/libsigrok-internal.h +++ b/src/libsigrok-internal.h @@ -574,10 +574,6 @@ struct sr_serial_dev_inst { char *serialcomm; /** libserialport port handle */ struct sp_port *data; - /** libserialport event set */ - struct sp_event_set *event_set; - /** GPollFDs for event polling */ - GPollFD *pollfds; }; #endif @@ -732,6 +728,9 @@ SR_PRIV int sr_session_source_remove_internal(struct sr_session *session, void *key); SR_PRIV int sr_session_source_destroyed(struct sr_session *session, void *key, GSource *source); +SR_PRIV int sr_session_fd_source_add(struct sr_session *session, + void *key, gintptr fd, int events, int timeout, + sr_receive_data_callback cb, void *cb_data); SR_PRIV int sr_session_send(const struct sr_dev_inst *sdi, const struct sr_datafeed_packet *packet); SR_PRIV int sr_sessionfile_check(const char *filename); diff --git a/src/serial.c b/src/serial.c index d9c611bd..136edcad 100644 --- a/src/serial.c +++ b/src/serial.c @@ -799,10 +799,17 @@ SR_PRIV int serial_source_add(struct sr_session *session, struct sr_serial_dev_inst *serial, int events, int timeout, sr_receive_data_callback cb, void *cb_data) { + struct sp_event_set *event_set; + gintptr poll_fd; + unsigned int poll_events; enum sp_event mask = 0; - unsigned int i; - if (sp_new_event_set(&serial->event_set) != SP_OK) + if ((events & (G_IO_IN|G_IO_ERR)) && (events & G_IO_OUT)) { + sr_err("Cannot poll input/error and output simultaneously."); + return SR_ERR_ARG; + } + + if (sp_new_event_set(&event_set) != SP_OK) return SR_ERR; if (events & G_IO_IN) @@ -812,54 +819,44 @@ SR_PRIV int serial_source_add(struct sr_session *session, if (events & G_IO_ERR) mask |= SP_EVENT_ERROR; - if (sp_add_port_events(serial->event_set, serial->data, mask) != SP_OK) { - sp_free_event_set(serial->event_set); + if (sp_add_port_events(event_set, serial->data, mask) != SP_OK) { + sp_free_event_set(event_set); return SR_ERR; } - - serial->pollfds = g_new0(GPollFD, serial->event_set->count); - - for (i = 0; i < serial->event_set->count; i++) { - - serial->pollfds[i].fd = (gintptr) - ((event_handle *)serial->event_set->handles)[i]; - mask = serial->event_set->masks[i]; - - if (mask & SP_EVENT_RX_READY) - serial->pollfds[i].events |= G_IO_IN; - if (mask & SP_EVENT_TX_READY) - serial->pollfds[i].events |= G_IO_OUT; - if (mask & SP_EVENT_ERROR) - serial->pollfds[i].events |= G_IO_ERR; - - if (sr_session_source_add_pollfd(session, &serial->pollfds[i], - timeout, cb, cb_data) != SR_OK) - return SR_ERR; + if (event_set->count != 1) { + sr_err("Unexpected number (%u) of event handles to poll.", + event_set->count); + sp_free_event_set(event_set); + return SR_ERR; } - return SR_OK; + poll_fd = (gintptr) ((event_handle *)event_set->handles)[0]; + mask = event_set->masks[0]; + + sp_free_event_set(event_set); + + poll_events = 0; + if (mask & SP_EVENT_RX_READY) + poll_events |= G_IO_IN; + if (mask & SP_EVENT_TX_READY) + poll_events |= G_IO_OUT; + if (mask & SP_EVENT_ERROR) + poll_events |= G_IO_ERR; + /* + * Using serial->data as the key for the event source is not quite + * proper, as it makes it impossible to create another event source + * for the same serial port. However, these fixed keys will soon be + * removed from the API anyway, so this is OK for now. + */ + return sr_session_fd_source_add(session, serial->data, + poll_fd, poll_events, timeout, cb, cb_data); } /** @private */ SR_PRIV int serial_source_remove(struct sr_session *session, struct sr_serial_dev_inst *serial) { - unsigned int i; - - if (!serial->event_set) - return SR_OK; - - for (i = 0; i < serial->event_set->count; i++) - if (sr_session_source_remove_pollfd(session, &serial->pollfds[i]) != SR_OK) - return SR_ERR; - - g_free(serial->pollfds); - sp_free_event_set(serial->event_set); - - serial->pollfds = NULL; - serial->event_set = NULL; - - return SR_OK; + return sr_session_source_remove_internal(session, serial->data); } /** diff --git a/src/session.c b/src/session.c index ad27a815..b8859021 100644 --- a/src/session.c +++ b/src/session.c @@ -985,8 +985,8 @@ SR_PRIV int sr_session_source_add_internal(struct sr_session *session, return ret; } -static int attach_fd_source(struct sr_session *session, - void *key, int fd, int events, int timeout, +SR_PRIV int sr_session_fd_source_add(struct sr_session *session, + void *key, gintptr fd, int events, int timeout, sr_receive_data_callback cb, void *cb_data) { GSource *source; @@ -1027,7 +1027,7 @@ SR_API int sr_session_source_add(struct sr_session *session, int fd, sr_err("Cannot create timer source without timeout."); return SR_ERR_ARG; } - return attach_fd_source(session, GINT_TO_POINTER(fd), + return sr_session_fd_source_add(session, GINT_TO_POINTER(fd), fd, events, timeout, cb, cb_data); } @@ -1054,7 +1054,7 @@ SR_API int sr_session_source_add_pollfd(struct sr_session *session, sr_err("%s: pollfd was NULL", __func__); return SR_ERR_ARG; } - return attach_fd_source(session, pollfd, pollfd->fd, + return sr_session_fd_source_add(session, pollfd, pollfd->fd, pollfd->events, timeout, cb, cb_data); } @@ -1093,7 +1093,7 @@ SR_API int sr_session_source_add_channel(struct sr_session *session, pollfd.fd = g_io_channel_unix_get_fd(channel); pollfd.events = events; #endif - return attach_fd_source(session, channel, pollfd.fd, + return sr_session_fd_source_add(session, channel, pollfd.fd, pollfd.events, timeout, cb, cb_data); }