[PATCH] serial-port: turn open_count assertion in mm_serial_port_close into simple return

[Aleksander Morgado]

On 11/01/14 01:54, Ben Chan wrote:
> When mm_serial_port_close is clearing a non-empty command queue, the
> response handler callback of a command could potentially call
> mm_serial_port_close again (e.g. via at_command_context_free,
> at_sequence_context_free in mm-base-modem-at.c), which triggers the
> following assertion:
> 
>   ModemManager: <debug> (ttyACM0) forced to close port
>   ModemManager: <debug> (ttyACM0) device open count is 0 (close)
>   ModemManager: <debug> (ttyACM0) closing serial port...
>   ModemManager: <debug> (ttyACM0) serial port closed
>   ModemManager: mm_serial_port_close: assertion `priv->open_count > 0' failed
> 
> The assertion is confusing as the condition is normal. This patch thus turns
> the 'priv->open_count > 0' assertion in mm_serial_port_close into a simple
> return.
> ---

I wouldn't say the condition is normal. Each port open request has its
corresponding port close request. The only case in which we should not
warn about close requests happening to a port with open_count==0 is when
the port was forced-closed, which is already handled for what I can see
"if (priv->forced_close) return;".

Now, I'm not sure in which situation you got that warning, but the
warning is indeed useful to detect issues with port open/close count
balance... The logs say that the ttyACM0 was forced-close; maybe
priv->forced_close wasn't set?


>  src/mm-serial-port.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mm-serial-port.c b/src/mm-serial-port.c
> index cc4fc46..ba29e8e 100644
> --- a/src/mm-serial-port.c
> +++ b/src/mm-serial-port.c
> @@ -1015,7 +1015,8 @@ mm_serial_port_close (MMSerialPort *self)
>      if (priv->forced_close)
>          return;
>  
> -    g_return_if_fail (priv->open_count > 0);
> +    if (priv->open_count == 0)
> +        return;
>  
>      device = mm_port_get_device (MM_PORT (self));
>  
> 


-- 
Aleksander Morgado
http://aleksander.es