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

[Ben Chan]

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  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;
>
>
>
___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list

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

[Dan Williams]

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;


___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list

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

[Ben Chan]

---
 src/mm-serial-port.c |7 +++
 1 files changed, 7 insertions(+), 0 deletions(-)

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;
-- 
1.7.7.3

___
networkmanager-list mailing list
networkmanager-list@gnome.org
https://mail.gnome.org/mailman/listinfo/networkmanager-list