Cleaner separation between imgXXX() functions and image_cmd.c

src/hci/commands/image_cmd.c

@@ -18,11 +18,11 @@
 
 #include <stdint.h>
 #include <stdlib.h>
+#include <libgen.h>
 #include <getopt.h>
 #include <vsprintf.h>
 #include <gpxe/image.h>
 #include <gpxe/command.h>
-#include <usr/fetch.h>
 #include <usr/imgmgmt.h>
 
 /** @file
@@ -37,36 +37,53 @@
  */
 
 /**
- * "fetch"/"module"/"kernel" command syntax message
+ * Fill in image command line
+ *
+ * @v image		Image
+ * @v nargs		Argument count
+ * @v args		Argument list
+ */
+void imgfill_cmdline ( struct image *image, unsigned int nargs, char **args ) {
+	size_t used = 0;
+
+	image->cmdline[0] = '\0';
+	while ( ( used < sizeof ( image->cmdline ) ) && nargs-- ) {
+		used += snprintf ( &image->cmdline[used],
+				   ( sizeof ( image->cmdline ) - used ),
+				   "%s%s", ( used ? " " : "" ), *(args++) );
+	}
+}
+
+/**
+ * "imgfetch"/"module"/"kernel" command syntax message
  *
  * @v argv		Argument list
  */
-static void fetch_syntax ( char **argv ) {
+static void imgfetch_core_syntax ( char **argv, int load ) {
 	printf ( "Usage:\n"
 		 "  %s [-n|--name <name>] filename [arguments...]\n"
 		 "\n"
-		 "Fetch executable/loadable image\n",
-		 argv[0] );
+		 "%s executable/loadable image\n",
+		 argv[0], ( load ? "Fetch and load" : "Fetch" ) );
 }
 
 /**
- * The "fetch"/"module"/"kernel" command body
+ * The "imgfetch"/"module"/"kernel" command body
  *
  * @v argc		Argument count
  * @v argv		Argument list
- * @v name		Default name for image, or NULL
+ * @v load		Load image after fetching
  * @ret rc		Exit code
  */
-static int fetch_exec_name ( int argc, char **argv, const char *name ) {
+static int imgfetch_core_exec ( int argc, char **argv, int load ) {
 	static struct option longopts[] = {
 		{ "help", 0, NULL, 'h' },
 		{ "name", required_argument, NULL, 'n' },
 		{ NULL, 0, NULL, 0 },
 	};
 	struct image *image;
-	const char *filename;
-	char cmdline[ sizeof ( image->cmdline ) ];
-	size_t used = 0;
+	const char *name = NULL;
+	char *filename;
 	int c;
 	int rc;
 
@@ -82,76 +99,246 @@ static int fetch_exec_name ( int argc, char **argv, const char *name ) {
 			/* Display help text */
 		default:
 			/* Unrecognised/invalid option */
-			fetch_syntax ( argv );
+			imgfetch_core_syntax ( argv, load );
 			return 1;
 		}
 	}
 
 	/* Need at least a filename remaining after the options */
-	if ( optind >= argc ) {
-		fetch_syntax ( argv );
+	if ( optind == argc ) {
+		imgfetch_core_syntax ( argv, load );
 		return 1;
 	}
 	filename = argv[optind++];
+	if ( ! name )
+		name = basename ( filename );
 
-	/* Build command line */
-	while ( ( used < sizeof ( cmdline ) ) && ( optind < argc ) ) {
-		used += snprintf ( &cmdline[used], sizeof ( cmdline ) - used,
-				   " %s",  argv[optind++] );
+	/* Fetch the image */
+	if ( ( rc = imgfetch ( filename, name, &image ) ) != 0 ) {
+		printf ( "Could not fetch %s: %s\n", name, strerror ( rc ) );
+		return 1;
 	}
 
-	/* Allocate and fill struct image */
-	image = malloc ( sizeof ( *image ) );
-	if ( ! image ) {
-		printf ( "Out of memory\n" );
-		return 1;
+	/* Fill in command line */
+	imgfill_cmdline ( image, ( argc - optind ), &argv[optind] );
+
+	/* Load image if required */
+	if ( load ) {
+		if ( ( rc = imgload ( image ) ) != 0 ) {
+			printf ( "Could not load %s: %s\n", name,
+				 strerror ( rc ) );
+			return 1;
+		}
 	}
-	memset ( image, 0, sizeof ( *image ) );
-	if ( name )
-		strncpy ( image->name, name, ( sizeof ( image->name ) - 1 ) );
-	if ( used )
-		memcpy ( image->cmdline, cmdline, sizeof ( image->cmdline ) );
-
-	/* Fetch the file */
-	if ( ( rc = fetch ( image, filename ) ) != 0 ) {
-		printf ( "Could not fetch %s: %s\n", filename,
-			 strerror ( rc ) );
-		free ( image );
+
+	return 0;
+}
+
+/**
+ * The "imgfetch"/"module" command
+ *
+ * @v argc		Argument count
+ * @v argv		Argument list
+ * @ret rc		Exit code
+ */
+static int imgfetch_exec ( int argc, char **argv ) {
+	return imgfetch_core_exec ( argc, argv, 0 );
+}
+
+/**
+ * The "kernel" command
+ *
+ * @v argc		Argument count
+ * @v argv		Argument list
+ * @ret rc		Exit code
+ */
+static int kernel_exec ( int argc, char **argv ) {
+	return imgfetch_core_exec ( argc, argv, 1  );
+}
+
+/**
+ * "imgload" command syntax message
+ *
+ * @v argv		Argument list
+ */
+static void imgload_syntax ( char **argv ) {
+	printf ( "Usage:\n"
+		 "  %s <image name>\n"
+		 "\n"
+		 "Load executable/loadable image\n",
+		 argv[0] );
+}
+
+/**
+ * The "imgload" command
+ *
+ * @v argc		Argument count
+ * @v argv		Argument list
+ * @ret rc		Exit code
+ */
+static int imgload_exec ( int argc, char **argv ) {
+	static struct option longopts[] = {
+		{ "help", 0, NULL, 'h' },
+		{ NULL, 0, NULL, 0 },
+	};
+	struct image *image;
+	const char *name;
+	int c;
+	int rc;
+
+	/* Parse options */
+	while ( ( c = getopt_long ( argc, argv, "h", longopts, NULL ) ) >= 0 ){
+		switch ( c ) {
+		case 'h':
+			/* Display help text */
+		default:
+			/* Unrecognised/invalid option */
+			imgload_syntax ( argv );
+			return 1;
+		}
+	}
+
+	/* Need exactly one image name remaining after the options */
+	if ( optind != ( argc - 1 ) ) {
+		imgload_syntax ( argv );
 		return 1;
 	}
+	name = argv[optind];
 
-	/* Register the image */
-	if ( ( rc = register_image ( image ) ) != 0 ) {
-		printf ( "Could not register %s: %s\n", filename,
-			 strerror ( rc ) );
-		free ( image );
+	/* Load all specified images */
+	image = find_image ( name );
+	if ( ! image ) {
+		printf ( "No such image: %s\n", name );
+		return 1;
+	}
+	if ( ( rc = imgload ( image ) ) != 0 ) {
+		printf ( "Could not load %s: %s\n", name, strerror ( rc ) );
 		return 1;
 	}
 
-	imgstat ( image );
 	return 0;
 }
 
 /**
- * The "fetch"/"module" command
+ * "imgargs" command syntax message
+ *
+ * @v argv		Argument list
+ */
+static void imgargs_syntax ( char **argv ) {
+	printf ( "Usage:\n"
+		 "  %s <image name> [<arguments>...]\n"
+		 "\n"
+		 "Set arguments for executable/loadable image\n",
+		 argv[0] );
+}
+
+/**
+ * The "imgargs" command body
  *
  * @v argc		Argument count
  * @v argv		Argument list
  * @ret rc		Exit code
  */
-static int fetch_exec ( int argc, char **argv ) {
-	return fetch_exec_name ( argc, argv, NULL );
+static int imgargs_exec ( int argc, char **argv ) {
+	static struct option longopts[] = {
+		{ "help", 0, NULL, 'h' },
+		{ NULL, 0, NULL, 0 },
+	};
+	struct image *image;
+	const char *name;
+	int c;
+
+	/* Parse options */
+	while ( ( c = getopt_long ( argc, argv, "h", longopts, NULL ) ) >= 0 ){
+		switch ( c ) {
+		case 'h':
+			/* Display help text */
+		default:
+			/* Unrecognised/invalid option */
+			imgargs_syntax ( argv );
+			return 1;
+		}
+	}
+
+	/* Need at least an image name remaining after the options */
+	if ( optind == argc ) {
+		imgargs_syntax ( argv );
+		return 1;
+	}
+	name = argv[optind++];
+
+	/* Fill in command line */
+	image = find_image ( name );
+	if ( ! image ) {
+		printf ( "No such image: %s\n", name );
+		return 1;
+	}
+	imgfill_cmdline ( image, ( argc - optind ), &argv[optind] );
+
+	return 0;
+}
+
+/**
+ * "imgexec" command syntax message
+ *
+ * @v argv		Argument list
+ */
+static void imgexec_syntax ( char **argv ) {
+	printf ( "Usage:\n"
+		 "  %s <image name>\n"
+		 "\n"
+		 "Execute executable/loadable image\n",
+		 argv[0] );
 }
 
 /**
- * The "kernel" command
+ * The "imgexec" command
  *
  * @v argc		Argument count
  * @v argv		Argument list
  * @ret rc		Exit code
  */
-static int kernel_exec ( int argc, char **argv ) {
-	return fetch_exec_name ( argc, argv, "kernel" );
+static int imgexec_exec ( int argc, char **argv ) {
+	static struct option longopts[] = {
+		{ "help", 0, NULL, 'h' },
+		{ NULL, 0, NULL, 0 },
+	};
+	struct image *image;
+	const char *name;
+	int c;
+	int rc;
+
+	/* Parse options */
+	while ( ( c = getopt_long ( argc, argv, "h", longopts, NULL ) ) >= 0 ){
+		switch ( c ) {
+		case 'h':
+			/* Display help text */
+		default:
+			/* Unrecognised/invalid option */
+			imgexec_syntax ( argv );
+			return 1;
+		}
+	}
+
+	/* Need exactly one image name */
+	if ( optind != ( argc - 1 ) ) {
+		imgexec_syntax ( argv );
+		return 1;
+	}
+	name = argv[optind];
+	
+	/* Execute specified image */
+	image = find_image ( name );
+	if ( ! image ) {
+		printf ( "No such image: %s\n", name );
+		return 1;
+	}
+	if ( ( rc = imgexec ( image ) ) != 0 ) {
+		printf ( "Could not execute %s: %s\n", name, strerror ( rc ) );
+		return 1;
+	}
+
+	return 0;
 }
 
 /**
@@ -174,7 +361,7 @@ static void imgstat_syntax ( char **argv ) {
  * @v argv		Argument list
  * @ret rc		Exit code
  */
-static int imgstat_exec ( int argc __unused, char **argv __unused ) {
+static int imgstat_exec ( int argc, char **argv ) {
 	static struct option longopts[] = {
 		{ "help", 0, NULL, 'h' },
 		{ NULL, 0, NULL, 0 },
@@ -194,7 +381,7 @@ static int imgstat_exec ( int argc __unused, char **argv __unused ) {
 		}
 	}
 
-	/* Need at least a filename remaining after the options */
+	/* No arguments */
 	if ( optind != argc ) {
 		imgstat_syntax ( argv );
 		return 1;
@@ -207,22 +394,93 @@ static int imgstat_exec ( int argc __unused, char **argv __unused ) {
 	return 0;
 }
 
+/**
+ * "imgstat" command syntax message
+ *
+ * @v argv		Argument list
+ */
+static void imgfree_syntax ( char **argv ) {
+	printf ( "Usage:\n"
+		 "  %s\n"
+		 "\n"
+		 "Free all executable/loadable images\n",
+		 argv[0] );
+}
+
+/**
+ * The "imgfree" command
+ *
+ * @v argc		Argument count
+ * @v argv		Argument list
+ * @ret rc		Exit code
+ */
+static int imgfree_exec ( int argc, char **argv ) {
+	static struct option longopts[] = {
+		{ "help", 0, NULL, 'h' },
+		{ NULL, 0, NULL, 0 },
+	};
+	struct image *image;
+	struct image *tmp;
+	int c;
+
+	/* Parse options */
+	while ( ( c = getopt_long ( argc, argv, "h", longopts, NULL ) ) >= 0 ){
+		switch ( c ) {
+		case 'h':
+			/* Display help text */
+		default:
+			/* Unrecognised/invalid option */
+			imgfree_syntax ( argv );
+			return 1;
+		}
+	}
+
+	/* No arguments */
+	if ( optind != argc ) {
+		imgfree_syntax ( argv );
+		return 1;
+	}
+
+	/* Free all images */
+	list_for_each_entry_safe ( image, tmp, &images, list ) {
+		imgfree ( image );
+	}
+	return 0;
+}
+
+
 /** Image management commands */
 struct command image_commands[] __command = {
 	{
-		.name = "fetch",
-		.exec = fetch_exec,
+		.name = "imgfetch",
+		.exec = imgfetch_exec,
 	},
 	{
 		.name = "module",
-		.exec = fetch_exec, /* synonym for "fetch" */
+		.exec = imgfetch_exec, /* synonym for "imgfetch" */
 	},
 	{
 		.name = "kernel",
 		.exec = kernel_exec,
 	},
+	{
+		.name = "imgload",
+		.exec = imgload_exec,
+	},
+	{
+		.name = "imgargs",
+		.exec = imgargs_exec,
+	},
+	{
+		.name = "imgexec",
+		.exec = imgexec_exec,
+	},
 	{
 		.name = "imgstat",
 		.exec = imgstat_exec,
 	},
+	{
+		.name = "imgfree",
+		.exec = imgfree_exec,
+	},
 };

src/include/usr/imgmgmt.h

@@ -7,6 +7,11 @@
  *
  */
 
+extern int imgfetch ( const char *filename, const char *name,
+		      struct image **new_image );
+extern int imgload ( struct image *image );
+extern int imgexec ( struct image *image );
 extern void imgstat ( struct image *image );
+extern void imgfree ( struct image *image );
 
 #endif /* _USR_IMGMGMT_H */

src/usr/fetch.c

@@ -23,7 +23,6 @@
  *
  */
 
-#include <libgen.h>
 #include <vsprintf.h>
 #include <gpxe/emalloc.h>
 #include <gpxe/ebuffer.h>
@@ -45,12 +44,6 @@ int fetch ( struct image *image, const char *filename ) {
 	struct buffer buffer;
 	int rc;
 
-	/* Name the image, if it isn't explicitly named */
-	if ( ! image->name[0] ) {
-		strncpy ( image->name, basename ( filename ),
-			  ( sizeof ( image->name ) - 1 ) );
-	}
-
 	/* Allocate an expandable buffer to hold the file */
 	if ( ( rc = ebuffer_alloc ( &buffer, 0 ) ) != 0 )
 		return rc;

src/usr/imgmgmt.c

@@ -17,8 +17,11 @@
  */
 
 #include <stdint.h>
+#include <stdlib.h>
+#include <errno.h>
 #include <vsprintf.h>
 #include <gpxe/image.h>
+#include <usr/fetch.h>
 #include <usr/imgmgmt.h>
 
 /** @file
@@ -27,19 +30,89 @@
  *
  */
 
+/**
+ * Fetch an image
+ *
+ * @v filename		Filename for image
+ * @v name		Name for image, or NULL
+ * @ret new_image	Newly created image
+ * @ret rc		Return status code
+ */
+int imgfetch ( const char *filename, const char *name,
+	       struct image **new_image ) {
+	struct image *image;
+	int rc;
+
+	/* Allocate new image */
+	image = malloc ( sizeof ( *image ) );
+	if ( ! image )
+		return -ENOMEM;
+	memset ( image, 0, sizeof ( *image ) );
+
+	/* Fill in image name */
+	if ( name )
+		strncpy ( image->name, name, ( sizeof ( image->name ) - 1 ) );
+
+	/* Fetch the file */
+	if ( ( rc = fetch ( image, filename ) ) != 0 )
+		goto err;
+
+	/* Register the image */
+	if ( ( rc = register_image ( image ) ) != 0 )
+		goto err;
+
+	*new_image = image;
+	return 0;
+
+ err:
+	free_image ( image );
+	free ( image );
+	return rc;
+}
+
+/**
+ * Load an image
+ *
+ * @v image		Image
+ * @ret rc		Return status code
+ */
+int imgload ( struct image *image ) {
+	return image_autoload ( image );
+}
+
+/**
+ * Execute an image
+ *
+ * @v image		Image
+ * @ret rc		Return status code
+ */
+int imgexec ( struct image *image ) {
+	return image_exec ( image );
+}
+
 /**
  * Display status of an image
  *
  * @v image		Executable/loadable image
  */
 void imgstat ( struct image *image ) {
-	printf ( "%s: %zd bytes ", image->name, image->len );
+	printf ( "%s: %zd bytes", image->name, image->len );
 	if ( image->type )
 		printf ( " [%s]", image->type->name );
 	if ( image->flags & IMAGE_LOADED )
 		printf ( " [LOADED]" );
 	if ( image->cmdline[0] )
-		printf ( "\"%s\"", image->cmdline );
+		printf ( " \"%s\"", image->cmdline );
 	printf ( "\n" );
 }
 
+/**
+ * Free an image
+ *
+ * @v image		Executable/loadable image
+ */
+void imgfree ( struct image *image ) {
+	unregister_image ( image );
+	free_image ( image );
+	free ( image );
+}