[comboot] Fix reference counting on replacement images
authorMichael Brown <mcb30@etherboot.org>
Tue, 17 Feb 2009 01:45:12 +0000 (01:45 +0000)
committerMichael Brown <mcb30@etherboot.org>
Tue, 17 Feb 2009 01:45:12 +0000 (01:45 +0000)
When chaining COMBOOT images, the old images now get freed correctly.

src/arch/i386/image/com32.c
src/arch/i386/image/comboot.c
src/arch/i386/interface/syslinux/comboot_call.c
src/core/image.c
src/include/gpxe/image.h

index 27ccbc0..3e81f48 100644 (file)
@@ -122,6 +122,7 @@ static int com32_exec ( struct image *image ) {
                DBGC ( image, "COM32 %p: exited to run kernel %p\n",
                       image, comboot_replacement_image );
                image->replacement = comboot_replacement_image;
+               comboot_replacement_image = NULL;
                image_autoload ( image->replacement );
                break;
 
index 3ca9d28..e045264 100644 (file)
@@ -191,6 +191,7 @@ static int comboot_exec ( struct image *image ) {
                DBGC ( image, "COMBOOT %p: exited to run kernel %p\n",
                       image, comboot_replacement_image );
                image->replacement = comboot_replacement_image;
+               comboot_replacement_image = NULL;
                image_autoload ( image->replacement );
                break;
 
index 02b2250..f37aa2a 100644 (file)
@@ -157,7 +157,7 @@ void comboot_force_text_mode ( void ) {
  * Fetch kernel and optional initrd
  */
 static int comboot_fetch_kernel ( char *kernel_file, char *cmdline ) {
-       struct image *kernel;
+       struct image *kernel = NULL;
        struct image *initrd = NULL;
        char *initrd_file;
        int rc;
@@ -181,13 +181,13 @@ static int comboot_fetch_kernel ( char *kernel_file, char *cmdline ) {
                if ( ! initrd ) {
                        DBG ( "COMBOOT: could not allocate initrd\n" );
                        rc = -ENOMEM;
-                       goto err_alloc_initrd;
+                       goto out;
                }
                if ( ( rc = imgfetch ( initrd, initrd_file,
                                       register_image ) ) != 0 ) {
                        DBG ( "COMBOOT: could not fetch initrd: %s\n",
                              strerror ( rc ) );
-                       goto err_fetch_initrd;
+                       goto out;
                }
 
                /* Restore space after initrd name, if applicable */
@@ -202,32 +202,31 @@ static int comboot_fetch_kernel ( char *kernel_file, char *cmdline ) {
        if ( ! kernel ) {
                DBG ( "COMBOOT: could not allocate kernel\n" );
                rc = -ENOMEM;
-               goto err_alloc_kernel;
+               goto out;
        }
        if ( ( rc = imgfetch ( kernel, kernel_file,
                               register_image ) ) != 0 ) {
                DBG ( "COMBOOT: could not fetch kernel: %s\n",
                      strerror ( rc ) );
-               goto err_fetch_kernel;
+               goto out;
        }
        if ( ( rc = image_set_cmdline ( kernel, cmdline ) ) != 0 ) {
                DBG ( "COMBOOT: could not set kernel command line: %s\n",
                      strerror ( rc ) );
-               goto err_set_cmdline;
+               goto out;
        }
 
        /* Store kernel as replacement image */
-       comboot_replacement_image = kernel;
-
-       return 0;
-
- err_set_cmdline:
- err_fetch_kernel:
+       assert ( comboot_replacement_image == NULL );
+       comboot_replacement_image = image_get ( kernel );
+
+ out:
+       /* Drop image references unconditionally; either we want to
+        * discard them, or they have been registered and we should
+        * drop out local reference.
+        */
        image_put ( kernel );
- err_alloc_kernel:
- err_fetch_initrd:
        image_put ( initrd );
- err_alloc_initrd:
        return rc;
 }
 
index 741b054..277d09a 100644 (file)
@@ -275,8 +275,7 @@ int image_exec ( struct image *image ) {
        /* Pick up replacement image before we drop the original
         * image's temporary reference.
         */
-       if ( ( replacement = image->replacement ) != NULL )
-               image_get ( replacement );
+       replacement = image->replacement;
 
        /* Drop temporary reference to the original image */
        image_put ( image );
index 0163b08..f8b1482 100644 (file)
@@ -53,7 +53,12 @@ struct image {
         * style similar to a Unix exec() call) should return from its
         * exec() method with the replacement image set to point to
         * the new image.  The new image must already be in a suitable
-        * state for execution.
+        * state for execution (i.e. loaded).
+        *
+        * If an image unregisters itself as a result of being
+        * executed, it must make sure that its replacement image (if
+        * any) is registered, otherwise the replacement is likely to
+        * be freed before it can be executed.
         */
        struct image *replacement;
 };