[image] Simplify image management

Refactor the {load,exec} image operations as {probe,exec}.  This makes
the probe mechanism cleaner, eliminates some forward declarations,
avoids holding magic state in image->priv, eliminates the possibility
of screwing up between the "load" and "exec" stages, and makes the
documentation simpler since the concept of "loading" (as distinct from
"executing") no longer needs to be explained.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 88 insertions, 94 deletions

diff --git a/src/core/image.c b/src/core/image.c
index ec4b4610..cb961531 100644
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -32,7 +32,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 
 /** @file
  *
- * Executable/loadable images
+ * Executable images
  *
  */
 
@@ -40,7 +40,7 @@ FILE_LICENCE ( GPL2_OR_LATER );
 struct list_head images = LIST_HEAD_INIT ( images );
 
 /**
- * Free executable/loadable image
+ * Free executable image
  *
  * @v refcnt		Reference counter
  */
@@ -52,13 +52,13 @@ static void free_image ( struct refcnt *refcnt ) {
 	ufree ( image->data );
 	image_put ( image->replacement );
 	free ( image );
-	DBGC ( image, "IMAGE %p freed\n", image );
+	DBGC ( image, "IMAGE %s freed\n", image->name );
 }
 
 /**
- * Allocate executable/loadable image
+ * Allocate executable image
  *
- * @ret image		Executable/loadable image
+ * @ret image		Executable image
  */
 struct image * alloc_image ( void ) {
 	struct image *image;
@@ -75,12 +75,11 @@ struct image * alloc_image ( void ) {
  *
  * @v image		Image
  * @v URI		New image URI
- * @ret rc		Return status code
  *
  * If no name is set, the name will be updated to the base name of the
  * URI path (if any).
  */
-int image_set_uri ( struct image *image, struct uri *uri ) {
+void image_set_uri ( struct image *image, struct uri *uri ) {
 	const char *path = uri->path;
 
 	/* Replace URI reference */
@@ -90,8 +89,6 @@ int image_set_uri ( struct image *image, struct uri *uri ) {
 	/* Set name if none already specified */
 	if ( path && ( ! image->name[0] ) )
 		image_set_name ( image, basename ( ( char * ) path ) );
-
-	return 0;
 }
 
 /**
@@ -110,9 +107,9 @@ int image_set_cmdline ( struct image *image, const char *cmdline ) {
 }
 
 /**
- * Register executable/loadable image
+ * Register executable image
  *
- * @v image		Executable/loadable image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
 int register_image ( struct image *image ) {
@@ -127,20 +124,20 @@ int register_image ( struct image *image ) {
 	/* Add to image list */
 	image_get ( image );
 	list_add_tail ( &image->list, &images );
-	DBGC ( image, "IMAGE %p at [%lx,%lx) registered as %s\n",
-	       image, user_to_phys ( image->data, 0 ),
-	       user_to_phys ( image->data, image->len ), image->name );
+	DBGC ( image, "IMAGE %s at [%lx,%lx) registered\n",
+	       image->name, user_to_phys ( image->data, 0 ),
+	       user_to_phys ( image->data, image->len ) );
 
 	return 0;
 }
 
 /**
- * Unregister executable/loadable image
+ * Unregister executable image
  *
- * @v image		Executable/loadable image
+ * @v image		Executable image
  */
 void unregister_image ( struct image *image ) {
-	DBGC ( image, "IMAGE %p unregistered\n", image );
+	DBGC ( image, "IMAGE %s unregistered\n", image->name );
 	list_del ( &image->list );
 	image_put ( image );
 }
@@ -149,7 +146,7 @@ void unregister_image ( struct image *image ) {
  * Find image by name
  *
  * @v name		Image name
- * @ret image		Executable/loadable image, or NULL
+ * @ret image		Executable image, or NULL
  */
 struct image * find_image ( const char *name ) {
 	struct image *image;
@@ -163,75 +160,39 @@ struct image * find_image ( const char *name ) {
 }
 
 /**
- * Load executable/loadable image into memory
- *
- * @v image		Executable/loadable image
- * @v type		Executable/loadable image type
- * @ret rc		Return status code
- */
-static int image_load_type ( struct image *image, struct image_type *type ) {
-	int rc;
-
-	/* Check image is actually loadable */
-	if ( ! type->load )
-		return -ENOEXEC;
-
-	/* Try the image loader */
-	if ( ( rc = type->load ( image ) ) != 0 ) {
-		DBGC ( image, "IMAGE %p could not load as %s: %s\n",
-		       image, type->name, strerror ( rc ) );
-		return rc;
-	}
-
-	/* Flag as loaded */
-	image->flags |= IMAGE_LOADED;
-	return 0;
-}
-
-/**
- * Load executable/loadable image into memory
- *
- * @v image		Executable/loadable image
- * @ret rc		Return status code
- */
-int image_load ( struct image *image ) {
-
-	assert ( image->type != NULL );
-
-	return image_load_type ( image, image->type );
-}
-
-/**
- * Autodetect image type and load executable/loadable image into memory
+ * Determine image type
  *
- * @v image		Executable/loadable image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
-int image_autoload ( struct image *image ) {
+int image_probe ( struct image *image ) {
 	struct image_type *type;
 	int rc;
 
-	/* If image already has a type, use it */
+	/* Succeed if we already have a type */
 	if ( image->type )
-		return image_load ( image );
+		return 0;
 
-	/* Otherwise probe for a suitable type */
+	/* Try each type in turn */
 	for_each_table_entry ( type, IMAGE_TYPES ) {
-		DBGC ( image, "IMAGE %p trying type %s\n", image, type->name );
-		rc = image_load_type ( image, type );
-		if ( image->type == NULL )
-			continue;
-		return rc;
+		if ( ( rc = type->probe ( image ) ) == 0 ) {
+			image->type = type;
+			DBGC ( image, "IMAGE %s is %s\n",
+			       image->name, type->name );
+			return 0;
+		}
+		DBGC ( image, "IMAGE %s is not %s: %s\n", image->name,
+		       type->name, strerror ( rc ) );
 	}
 
-	DBGC ( image, "IMAGE %p format not recognised\n", image );
+	DBGC ( image, "IMAGE %s format not recognised\n", image->name );
 	return -ENOEXEC;
 }
 
 /**
- * Execute loaded image
+ * Execute image
  *
- * @v image		Loaded image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
 int image_exec ( struct image *image ) {
@@ -239,18 +200,9 @@ int image_exec ( struct image *image ) {
 	struct uri *old_cwuri;
 	int rc;
 
-	/* Image must be loaded first */
-	if ( ! ( image->flags & IMAGE_LOADED ) ) {
-		DBGC ( image, "IMAGE %p could not execute: not loaded\n",
-		       image );
-		return -ENOTTY;
-	}
-
-	assert ( image->type != NULL );
-
-	/* Check that image is actually executable */
-	if ( ! image->type->exec )
-		return -ENOEXEC;
+	/* Check that this image can be executed */
+	if ( ( rc = image_probe ( image ) ) != 0 )
+		return rc;
 
 	/* Switch current working directory to be that of the image itself */
 	old_cwuri = uri_get ( cwuri );
@@ -264,8 +216,8 @@ int image_exec ( struct image *image ) {
 
 	/* Try executing the image */
 	if ( ( rc = image->type->exec ( image ) ) != 0 ) {
-		DBGC ( image, "IMAGE %p could not execute: %s\n",
-		       image, strerror ( rc ) );
+		DBGC ( image, "IMAGE %s could not execute: %s\n",
+		       image->name, strerror ( rc ) );
 		/* Do not return yet; we still have clean-up to do */
 	}
 
@@ -283,8 +235,8 @@ int image_exec ( struct image *image ) {
 
 	/* Tail-recurse into replacement image, if one exists */
 	if ( replacement ) {
-		DBGC ( image, "IMAGE %p replacing self with IMAGE %p\n",
-		       image, replacement );
+		DBGC ( image, "IMAGE %s replacing self with IMAGE %s\n",
+		       image->name, replacement->name );
 		if ( ( rc = image_exec ( replacement ) ) != 0 )
 			return rc;
 	}
@@ -293,33 +245,75 @@ int image_exec ( struct image *image ) {
 }
 
 /**
- * Register and autoload an image
+ * Select image for execution
  *
- * @v image		Image
+ * @v image		Executable image
  * @ret rc		Return status code
  */
-int register_and_autoload_image ( struct image *image ) {
+int image_select ( struct image *image ) {
+	struct image *tmp;
+	int rc;
+
+	/* Unselect all other images */
+	for_each_image ( tmp )
+		tmp->flags &= ~IMAGE_SELECTED;
+
+	/* Check that this image can be executed */
+	if ( ( rc = image_probe ( image ) ) != 0 )
+		return rc;
+
+	/* Mark image as selected */
+	image->flags |= IMAGE_SELECTED;
+
+	return 0;
+}
+
+/**
+ * Find selected image
+ *
+ * @ret image		Executable image, or NULL
+ */
+struct image * image_find_selected ( void ) {
+	struct image *image;
+
+	for_each_image ( image ) {
+		if ( image->flags & IMAGE_SELECTED )
+			return image;
+	}
+	return NULL;
+}
+
+/**
+ * Register and select an image
+ *
+ * @v image		Executable image
+ * @ret rc		Return status code
+ */
+int register_and_select_image ( struct image *image ) {
 	int rc;
 
 	if ( ( rc = register_image ( image ) ) != 0 )
 		return rc;
 
-	if ( ( rc = image_autoload ( image ) ) != 0 )
+	if ( ( rc = image_probe ( image ) ) != 0 )
+		return rc;
+
+	if ( ( rc = image_select ( image ) ) != 0 )
 		return rc;
 
 	return 0;
 }
 
 /**
- * Register and autoexec an image
+ * Register and boot an image
  *
  * @v image		Image
  * @ret rc		Return status code
  */
-int register_and_autoexec_image ( struct image *image ) {
+int register_and_boot_image ( struct image *image ) {
 	int rc;
 
-	if ( ( rc = register_and_autoload_image ( image ) ) != 0 )
+	if ( ( rc = register_and_select_image ( image ) ) != 0 )
 		return rc;
 
 	if ( ( rc = image_exec ( image ) ) != 0 )