Re: [MM] [PATCH] serial-port: avoid opening a serial port that has been disposed

[Ben Chan <benchan chromium org>]

I guess the major concern would be using priv->forced_close as an indicator of whether the serial port is allowed to reopen or not. How about using another variable to tag the MMSerialPort object as being disposed?  There are a few call sites calling mm_serial_port_open(), so it seems better to check inside mm_serial_port_open().

How do you think?  I can revise the patch if that makes sense.

Thanks,

Ben

On Tue, Nov 27, 2012 at 12:03 PM, Dan Williams <dcbw redhat com> wrote:

On Tue, 2012-11-27 at 10:57 -0800, Ben Chan wrote:
> ---
>  src/mm-serial-port.c |    7 +++++++
>  1 files changed, 7 insertions(+), 0 deletions(-)

I guess I'd rather fix the code that's attempting to re-open the port if
it's not supposed to do that.  This has the effect of making ports
single-use only, which wasn't originally the intention.  Forcing a port
closed regardless of the open count doesn't necessarily mean the port
object itself has been disposed.

Dan

> diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
> index 0a8820d..dee2fec 100644
> --- a/src/mm-serial-port.c
> +++ b/src/mm-serial-port.c
> @@ -849,6 +849,13 @@ mm_serial_port_open (MMSerialPort *self, GError **error)
>
>      device = mm_port_get_device (MM_PORT (self));
>
> +    /* If we forced closing the port, the MMSerialPort object has been disposed.
> +     * Just return an error. */
> +    if (priv->forced_close) {
> +        mm_info ("(%s) skipped opening serial port that has been disposed", device);
> +        return FALSE;
> +    }
> +
>      if (priv->open_count) {
>          /* Already open */
>          goto success;