Do not hold self-references. This then avoids the problem of having to
authorMichael Brown <mcb30@etherboot.org>
Tue, 15 May 2007 16:53:46 +0000 (16:53 +0000)
committerMichael Brown <mcb30@etherboot.org>
Tue, 15 May 2007 16:53:46 +0000 (16:53 +0000)
ensure that we only drop our self-reference exactly once.

To maintain the guarantee that an object won't go out of scope
unexpectedly while one of its event handlers is being called, the
event-calling functions now automatically obtain and drop extra
references.

src/core/downloader.c
src/core/hw.c
src/core/job.c
src/core/xfer.c
src/include/gpxe/interface.h
src/include/gpxe/job.h
src/include/gpxe/xfer.h

index 2e466cb..653a480 100644 (file)
@@ -50,6 +50,19 @@ struct downloader {
        int ( * register_image ) ( struct image *image );
 };
 
+/**
+ * Free downloader object
+ *
+ * @v refcnt           Downloader reference counter
+ */
+static void downloader_free ( struct refcnt *refcnt ) {
+       struct downloader *downloader =
+               container_of ( refcnt, struct downloader, refcnt );
+
+       image_put ( downloader->image );
+       free ( downloader );
+}
+
 /**
  * Terminate download
  *
@@ -63,12 +76,8 @@ static void downloader_finished ( struct downloader *downloader, int rc ) {
        xfer_nullify ( &downloader->xfer );
 
        /* Free resources and close interfaces */
-       image_put ( downloader->image );
        xfer_close ( &downloader->xfer, rc );
        job_done ( &downloader->job, rc );
-
-       /* Drop reference to self */
-       ref_put ( &downloader->refcnt );
 }
 
 /**
@@ -267,6 +276,7 @@ int create_downloader ( struct job_interface *job, const char *uri_string,
        if ( ! downloader )
                return -ENOMEM;
        memset ( downloader, 0, sizeof ( *downloader ) );
+       downloader->refcnt.free = downloader_free;
        job_init ( &downloader->job, &downloader_job_operations,
                   &downloader->refcnt );
        xfer_init ( &downloader->xfer, &downloader_xfer_operations,
@@ -279,11 +289,13 @@ int create_downloader ( struct job_interface *job, const char *uri_string,
                           uri_string ) ) != 0 )
                goto err;
 
-       /* Attach parent interface and return */
+       /* Attach parent interface, mortalise self, and return */
        job_plug_plug ( &downloader->job, job );
+       ref_put ( &downloader->refcnt );
        return 0;
 
  err:
        downloader_finished ( downloader, rc );
+       ref_put ( &downloader->refcnt );
        return rc;
 }
index 80cac99..460ec58 100644 (file)
@@ -22,7 +22,6 @@ static const char hw_msg[] = "Hello world!\n";
 static void hw_finished ( struct hw *hw, int rc ) {
        xfer_nullify ( &hw->xfer );
        xfer_close ( &hw->xfer, rc );
-       ref_put ( &hw->refcnt );
 }
 
 static void hw_xfer_close ( struct xfer_interface *xfer, int rc ) {
@@ -31,13 +30,14 @@ static void hw_xfer_close ( struct xfer_interface *xfer, int rc ) {
        hw_finished ( hw, rc );
 }
 
-static int hw_xfer_request ( struct xfer_interface *xfer, off_t start __unused,
-                            int whence __unused, size_t len __unused ) {
+static void hw_xfer_request ( struct xfer_interface *xfer,
+                             off_t start __unused, int whence __unused,
+                             size_t len __unused ) {
        struct hw *hw = container_of ( xfer, struct hw, xfer );
+       int rc;
 
-       xfer_deliver_raw ( xfer, hw_msg, sizeof ( hw_msg ) );
-       hw_finished ( hw, 0 );
-       return 0;
+       rc = xfer_deliver_raw ( xfer, hw_msg, sizeof ( hw_msg ) );
+       hw_finished ( hw, rc );
 }
 
 static struct xfer_interface_operations hw_xfer_operations = {
@@ -52,13 +52,16 @@ static struct xfer_interface_operations hw_xfer_operations = {
 static int hw_open ( struct xfer_interface *xfer, struct uri *uri __unused ) {
        struct hw *hw;
 
+       /* Allocate and initialise structure */
        hw = malloc ( sizeof ( *hw ) );
        if ( ! hw )
                return -ENOMEM;
        memset ( hw, 0, sizeof ( *hw ) );
        xfer_init ( &hw->xfer, &hw_xfer_operations, &hw->refcnt );
 
+       /* Attach parent interface, mortalise self, and return */
        xfer_plug_plug ( &hw->xfer, xfer );
+       ref_put ( &hw->refcnt );
        return 0;
 }
 
index 7a0e0ee..1c589fc 100644 (file)
  */
 
 void job_done ( struct job_interface *job, int rc ) {
-       struct job_interface *dest = job_dest ( job );
+       struct job_interface *dest = job_get_dest ( job );
 
        dest->op->done ( dest, rc );
        job_unplug ( job );
+       job_put ( dest );
 }
 
 /****************************************************************************
index bfbf1ce..f2783f5 100644 (file)
  * @v rc               Reason for close
  */
 void xfer_close ( struct xfer_interface *xfer, int rc ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
 
        dest->op->close ( dest, rc );
        xfer_unplug ( xfer );
+       xfer_put ( dest );
 }
 
 /**
@@ -48,9 +49,12 @@ void xfer_close ( struct xfer_interface *xfer, int rc ) {
  * @ret rc             Return status code
  */
 int xfer_vredirect ( struct xfer_interface *xfer, int type, va_list args ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
+       int rc;
 
-       return dest->op->vredirect ( dest, type, args );
+       rc = dest->op->vredirect ( dest, type, args );
+       xfer_put ( dest );
+       return rc;
 }
 
 /**
@@ -82,9 +86,12 @@ int xfer_redirect ( struct xfer_interface *xfer, int type, ... ) {
  */
 int xfer_request ( struct xfer_interface *xfer, off_t offset, int whence,
                   size_t len ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
+       int rc;
 
-       return dest->op->request ( dest, offset, whence, len );
+       rc = dest->op->request ( dest, offset, whence, len );
+       xfer_put ( dest );
+       return rc;
 }
 
 /**
@@ -106,9 +113,12 @@ int xfer_request_all ( struct xfer_interface *xfer ) {
  * @ret rc             Return status code
  */
 int xfer_seek ( struct xfer_interface *xfer, off_t offset, int whence ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
+       int rc;
 
-       return dest->op->seek ( dest, offset, whence );
+       rc = dest->op->seek ( dest, offset, whence );
+       xfer_put ( dest );
+       return rc;
 }
 
 /**
@@ -119,9 +129,12 @@ int xfer_seek ( struct xfer_interface *xfer, off_t offset, int whence ) {
  * @ret iobuf          I/O buffer
  */
 struct io_buffer * xfer_alloc_iob ( struct xfer_interface *xfer, size_t len ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
+       struct io_buffer *iobuf;
 
-       return dest->op->alloc_iob ( dest, len );
+       iobuf = dest->op->alloc_iob ( dest, len );
+       xfer_put ( dest );
+       return iobuf;
 }
 
 /**
@@ -132,9 +145,12 @@ struct io_buffer * xfer_alloc_iob ( struct xfer_interface *xfer, size_t len ) {
  * @ret rc             Return status code
  */
 int xfer_deliver_iob ( struct xfer_interface *xfer, struct io_buffer *iobuf ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
+       int rc;
 
-       return dest->op->deliver_iob ( dest, iobuf );
+       rc = dest->op->deliver_iob ( dest, iobuf );
+       xfer_put ( dest );
+       return rc;
 }
 
 /**
@@ -146,9 +162,12 @@ int xfer_deliver_iob ( struct xfer_interface *xfer, struct io_buffer *iobuf ) {
  */
 int xfer_deliver_raw ( struct xfer_interface *xfer,
                       const void *data, size_t len ) {
-       struct xfer_interface *dest = xfer_dest ( xfer );
+       struct xfer_interface *dest = xfer_get_dest ( xfer );
+       int rc;
 
-       return dest->op->deliver_raw ( dest, data, len );
+       rc = dest->op->deliver_raw ( dest, data, len );
+       xfer_put ( dest );
+       return rc;
 }
 
 /****************************************************************************
index d27ba6a..94c711a 100644 (file)
@@ -34,7 +34,8 @@ struct interface {
  * @v intf             Interface
  * @ret intf           Interface
  */
-static inline struct interface * intf_get ( struct interface *intf ) {
+static inline __attribute__ (( always_inline )) struct interface *
+intf_get ( struct interface *intf ) {
        ref_get ( intf->refcnt );
        return intf;
 }
@@ -44,7 +45,8 @@ static inline struct interface * intf_get ( struct interface *intf ) {
  *
  * @v intf             Interface
  */
-static inline void intf_put ( struct interface *intf ) {
+static inline __attribute__ (( always_inline )) void
+intf_put ( struct interface *intf ) {
        ref_put ( intf->refcnt );
 }
 
index 2b33408..2888586 100644 (file)
@@ -104,14 +104,24 @@ intf_to_job ( struct interface *intf ) {
 }
 
 /**
- * Get destination job control interface
+ * Get reference to destination job control interface
  *
  * @v job              Job control interface
  * @ret dest           Destination interface
  */
 static inline __attribute__ (( always_inline )) struct job_interface *
-job_dest ( struct job_interface *job ) {
-       return intf_to_job ( job->intf.dest );
+job_get_dest ( struct job_interface *job ) {
+       return intf_to_job ( intf_get ( job->intf.dest ) );
+}
+
+/**
+ * Drop reference to job control interface
+ *
+ * @v job              Job control interface
+ */
+static inline __attribute__ (( always_inline )) void
+job_put ( struct job_interface *job ) {
+       intf_put ( &job->intf );
 }
 
 /**
index 4fb3a51..71b69dc 100644 (file)
@@ -173,14 +173,24 @@ intf_to_xfer ( struct interface *intf ) {
 }
 
 /**
- * Get destination data transfer interface
+ * Get reference to destination data transfer interface
  *
  * @v xfer             Data transfer interface
  * @ret dest           Destination interface
  */
 static inline __attribute__ (( always_inline )) struct xfer_interface *
-xfer_dest ( struct xfer_interface *xfer ) {
-       return intf_to_xfer ( xfer->intf.dest );
+xfer_get_dest ( struct xfer_interface *xfer ) {
+       return intf_to_xfer ( intf_get ( xfer->intf.dest ) );
+}
+
+/**
+ * Drop reference to data transfer interface
+ *
+ * @v xfer             Data transfer interface
+ */
+static inline __attribute__ (( always_inline )) void
+xfer_put ( struct xfer_interface *xfer ) {
+       intf_put ( &xfer->intf );
 }
 
 /**
@@ -189,8 +199,8 @@ xfer_dest ( struct xfer_interface *xfer ) {
  * @v xfer             Data transfer interface
  * @v dest             New destination interface
  */
-static inline void xfer_plug ( struct xfer_interface *xfer,
-                              struct xfer_interface *dest ) {
+static inline __attribute__ (( always_inline )) void
+xfer_plug ( struct xfer_interface *xfer, struct xfer_interface *dest ) {
        plug ( &xfer->intf, &dest->intf );
 }
 
@@ -200,8 +210,8 @@ static inline void xfer_plug ( struct xfer_interface *xfer,
  * @v a                        Data transfer interface A
  * @v b                        Data transfer interface B
  */
-static inline void xfer_plug_plug ( struct xfer_interface *a,
-                                   struct xfer_interface *b ) {
+static inline __attribute__ (( always_inline )) void
+xfer_plug_plug ( struct xfer_interface *a, struct xfer_interface *b ) {
        plug_plug ( &a->intf, &b->intf );
 }
 
@@ -210,7 +220,8 @@ static inline void xfer_plug_plug ( struct xfer_interface *a,
  *
  * @v xfer             Data transfer interface
  */
-static inline void xfer_unplug ( struct xfer_interface *xfer ) {
+static inline __attribute__ (( always_inline )) void
+xfer_unplug ( struct xfer_interface *xfer ) {
        plug ( &xfer->intf, &null_xfer.intf );
 }