[LinuxBIOS] [patch 0/5] LAR improvements (V2)

Hey all.  Following is a refresh of the LAR improvements that I posted
earlier.  This revision includes fixes for comments from the mailing list,
bugs I discovered while integrating the code into buildROM, and last
(but not least), Doxygen style comments.  I know some of this code got
ACKed last time I sent them out, but the messages were flying about 
left and right, and I figured it would just be easier to re-send them all.
Also, not pictured here is the "remove-files" patch - I'll integrate it
with the add-stream.patch when I commit the code, but since it just 
removes code, it isn't interesting to you guys.

Keep an eye out for the buildROM v3 enhancements, coming soon to a 
mailing list near you.  Following that, I have some plans for future
LAR work - including the manifest stuff we discussed earlier, as well
as possible replace/delete support in the LAR utility.

Enjoy,
Jordan

--
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 1/5] Add a top level target for LAR

Add a top level target to the LAR makefile so it can be built by itself
in the LBv3 tree.  Also remove any reference to the build system so that
LAr can be constructed in an un-configured tree.

Signed-off-by: Jordan Crouse <[e-mail]>
Index: LinuxBIOSv3/util/lar/Makefile
===================================================================
--- LinuxBIOSv3.orig/util/lar/Makefile	2007-07-23 09:44:22.000000000 -0600
+++ LinuxBIOSv3/util/lar/Makefile	2007-07-23 09:44:26.000000000 -0600
@@ -47,6 +47,8 @@
 	$(Q)$(HOSTCC) $(HOSTCFLAGS) -c $< -o $@
 
 
+lar: $(obj)/util/lar/lar
+
 # -----------------------------------------------------------------------------
 # Stuff below this line is for debugging purposes only.
 
Index: LinuxBIOSv3/util/lar/lar.h
===================================================================
--- LinuxBIOSv3.orig/util/lar/lar.h	2007-07-23 09:44:21.000000000 -0600
+++ LinuxBIOSv3/util/lar/lar.h	2007-07-23 09:44:34.000000000 -0600
@@ -49,7 +49,6 @@
  */
 
 #include <stdint.h>
-#include "../../build/config.h"
 
 #define MAGIC "LARCHIVE"
 #define MAX_PATHLEN 1024

--
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 1/5] Add a top level target for LAR

[Peter Stuge]

On Mon, Jul 23, 2007 at 10:06:03AM -0600, [e-mail] wrote:
> Add a top level target to the LAR makefile so it can be built by itself
> in the LBv3 tree.  Also remove any reference to the build system so that
> LAr can be constructed in an un-configured tree.

Would it be simple to allow in-tree building?

Also, this is not a good default:

~/lb/LinuxBIOSv3/util/lar $ make lar
printf "  BUILD   LAR\n"
  BUILD   LAR
mkdir -p /util/lar
mkdir: cannot create directory /util': Permission denied
make: *** [lardir] Error 1


How can we handle it better?


//Peter

[LinuxBIOS] [patch 2/5] New LAR access functions

In preparation for adding new LAR functionality - this patch consolidates
creating and accessing the LAR into new code utilizing mmap which
facilitates moving about within the archive. 

This code also turns the bootblock path name as a constant value.
It also requires that the user specify a size when the LAR is
created.

Signed-off-by: Jordan crouse <[e-mail]>
Index: LinuxBIOSv3/util/lar/Makefile
===================================================================
--- LinuxBIOSv3.orig/util/lar/Makefile	2007-07-23 09:46:54.000000000 -0600
+++ LinuxBIOSv3/util/lar/Makefile	2007-07-23 09:47:04.000000000 -0600
@@ -18,7 +18,7 @@
 ## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
 ##
 
-LAROBJ := lar.o create.o extract.o list.o lib.o bootblock.o
+LAROBJ := lar.o stream.o lib.o
 
 LARDIR := lardir
 
Index: LinuxBIOSv3/util/lar/lar.c
===================================================================
--- LinuxBIOSv3.orig/util/lar/lar.c	2007-07-23 09:45:39.000000000 -0600
+++ LinuxBIOSv3/util/lar/lar.c	2007-07-23 09:47:04.000000000 -0600
@@ -62,6 +62,64 @@
 	return bootblock;
 }
 
+int create_lar(const char *archivename, struct file *files)
+{
+	struct lar *lar = lar_new_archive(archivename, larsize);
+
+	if (lar == NULL) {
+		fprintf(stderr, "Unable to create %s as a LAR archive.\n",
+			archivename);
+		exit(1);
+	}
+
+	for( ; files; files = files->next) {
+		if (lar_add_file(lar, files->name)) {
+			fprintf(stderr, "Error adding %s to the LAR.\n", files->name);
+			lar_close_archive(lar);
+			exit(1);
+		}
+	}
+
+	if (lar_add_bootblock(lar, bootblock)) {
+		fprintf(stderr, "Error adding the bootblock to the LAR.\n");
+		lar_close_archive(lar);
+		exit(1);
+	}
+
+	lar_close_archive(lar);
+	return 0;
+}
+
+int list_lar(const char *archivename, struct file *files)
+{
+	struct lar *lar = lar_open_archive(archivename);
+
+	if (lar == NULL) {
+		fprintf(stderr, "Unable to open LAR archive %s\n", archivename);
+		exit(1);
+	}
+
+	lar_list_files(lar, files);
+	lar_close_archive(lar);
+	return 0;
+}
+
+int extract_lar(const char *archivename, struct file *files)
+{
+	int ret;
+
+	struct lar *lar = lar_open_archive(archivename);
+
+	if (lar == NULL) {
+		fprintf(stderr, "Unable to open LAR archive %s\n", archivename);
+		exit(1);
+	}
+
+	ret = lar_extract_files(lar, files);
+	lar_close_archive(lar);
+	return ret;
+}
+
 int main(int argc, char *argv[])
 {
 	int opt;
@@ -173,16 +231,9 @@
 
 		/* adding a bootblock only makes sense when creating a lar */
 		if (!larsize) {
-			printf("Warning: When specifying a bootblock "
-			       "you should also set an archive size.\n");
-		}
-
-		/* load the bootblock */
-		if (larmode == CREATE) {
-			load_bootblock(bootblock);
-			fixup_bootblock();
+			printf("When creating a LAR archive, you must specify an archive size.\n");
+			exit(1);
 		}
-
 	}
 
 	if (optind < argc) {
Index: LinuxBIOSv3/util/lar/lar.h
===================================================================
--- LinuxBIOSv3.orig/util/lar/lar.h	2007-07-23 09:46:54.000000000 -0600
+++ LinuxBIOSv3/util/lar/lar.h	2007-07-23 09:47:04.000000000 -0600
@@ -54,7 +54,11 @@
 #define MAX_PATHLEN 1024
 #define BOOTBLOCK_SIZE 16384
 
+#define BOOTBLOCK_NAME "bootblock"
+#define BOOTBLOCK_NAME_LEN 16
+
 typedef uint32_t u32;
+typedef uint8_t  u8;
 
 struct lar_header {
 	char magic[8];
@@ -71,6 +75,16 @@
 	u32 compression;
 };
 
+/**\struct
+ * A structure containing information about a currently open and mapped
+ * LAR archive
+ */
+struct lar {
+	int fd;   /**< The file descriptor of the open archive */
+	u8 *map;  /**< A pointer to the mmap()ed file */
+	u32 size; /**< Size of the mmaped file */
+};
+
 enum compalgo { none = 0, lzma = 1, nrv2b = 2 };
 
 typedef void (*compress_func) (char *, u32, char *, u32 *);
Index: LinuxBIOSv3/util/lar/stream.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ LinuxBIOSv3/util/lar/stream.c	2007-07-23 09:47:04.000000000 -0600
@@ -0,0 +1,661 @@
+/*
+ * lar - LinuxBIOS archiver
+ *
+ * This includes code from previous versions of the LAR utility,
+ * including create.c, list.c, extract.c and bootblock.c
+ *
+ * Copyright (C) 2006-2007 coresystems GmbH
+ * (Written by Stefan Reinauer <[e-mail]> for coresystems GmbH)
+ * Copyright (C) 2007 Patrick Georgi <[e-mail]>
+ * Copyright (C) 2007 Advanced Micro Devices, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
+ */
+
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <netinet/in.h>
+#include <libgen.h>
+
+#include "lib.h"
+#include "lar.h"
+
+/**
+ * \def err(fmt,args...)
+ * Abstract outputing error strings to avoid repetition
+ */
+#define err(fmt,args...) fprintf(stderr, fmt, ##args)
+
+extern enum compalgo algo;
+
+/**
+ * Given a size, return the offset of the bootblock (including the
+ * header)
+ * @param size Size of the LAR archive
+ * @return The offset of the bootblock header
+ */
+static inline u32 get_bootblock_offset(u32 size)
+{
+	return size - (BOOTBLOCK_SIZE + sizeof(struct lar_header)
+		       + BOOTBLOCK_NAME_LEN);
+}
+
+/**
+ * Return the expected offset of the next LAR header after the given one
+ * @param header The current LAR header
+ * @return The offset of the next possible LAR header
+ */
+static inline u32 get_next_offset(struct lar_header *header) {
+	return ((ntohl(header->len) + ntohl(header->offset) - 1) & 0xFFFFFFF0)
+		+ 16;
+}
+
+/**
+ * Read the size of the LAR archive from the size embedded in the bootblock
+ * @param lar The LAR archive to read from
+ * @return The size as read from the bootblock header
+ */
+static int lar_read_size(struct lar *lar)
+{
+	u32 *ptr = (u32 *) (lar->map + (lar->size - 12));
+	return ptr[0];
+}
+
+/**
+ * Add the LAR archive size to the bootblock, and clean up some other params
+ * in what we're loosely calling the "bootblockh header"
+ * @param ptr Pointer to the start of the bootblock
+ * @param size The size value to write to the bootblock header
+ */
+static void annotate_bootblock(u8 *ptr, u32 size)
+{
+	int i;
+	u32 *p;
+
+	memset(ptr + (BOOTBLOCK_SIZE - 13), 0, 13);
+	p = (u32 *) (ptr + BOOTBLOCK_SIZE - 12);
+	p[0] = size;
+}
+
+/**
+ * Add a bootblock file to the LAR - the bootblock must be of a consistant
+ * size, and always gets put in a special location in the LAR
+ * @param lar The LAR archive to write to
+ * @param bootblock The name of the bootblock file to insert
+ * @return 0 on success, or -1 on failure
+ */
+int lar_add_bootblock(struct lar *lar, const char *bootblock)
+{
+	u8 *offset;
+	struct lar_header *header;
+	int ret;
+
+	offset = lar->map + get_bootblock_offset(lar->size);
+	header = (struct lar_header *) offset;
+
+	memcpy(header->magic, MAGIC, 8);
+	header->reallen = htonl(BOOTBLOCK_SIZE);
+	header->len = htonl(BOOTBLOCK_SIZE);
+	header->offset = htonl(sizeof(struct lar_header) + BOOTBLOCK_NAME_LEN);
+
+	offset += sizeof(struct lar_header);
+	strcpy((char *) offset, BOOTBLOCK_NAME);
+
+	offset += BOOTBLOCK_NAME_LEN;
+
+	/* If a file waas specified, then load it, and read it directly
+	 * into place */
+
+	if (bootblock != NULL) {
+		struct stat s;
+		int fd = open(bootblock, O_RDONLY);
+
+		if (fd == -1) {
+			err("Unable to read bootblock file %s\n", bootblock);
+			return -1;
+		}
+
+	        ret = fstat(fd, &s);
+
+		if (ret == -1) {
+			err("Unable to stat %s\n", bootblock);
+			close(fd);
+			return -1;
+		}
+
+		if (s.st_size != BOOTBLOCK_SIZE) {
+			err("Bootblock %s does not appear to be a bootblock.\n",
+			    bootblock);
+			close(fd);
+			return -1;
+		}
+
+		ret = read(fd, offset, BOOTBLOCK_SIZE);
+		close(fd);
+
+		if (ret != BOOTBLOCK_SIZE) {
+			err("Unable to read all the bytes in the bootblock file.\n");
+			return -1;
+		}
+	}
+
+	annotate_bootblock(offset, lar->size);
+	return 0;
+}
+
+/**
+ * mmap() the LAR archive
+ * @param lar The LAR archive information to map
+ * @param u32 size Size of the mapped region
+ * @return 0 on success, or -1 on failure
+ */
+static int _map_lar(struct lar *lar, u32 size)
+{
+	if (lar == NULL)
+		return -1;
+
+	lar->map = mmap(0, size, PROT_READ | PROT_WRITE, MAP_SHARED,
+			lar->fd, 0);
+
+	lar->size = size;
+
+	if (lar->map == MAP_FAILED)
+		return -1;
+
+	return 0;
+}
+
+/**
+ * Close the LAR archive and free all structures
+ * @param lar The LAR archive to close
+ */
+void lar_close_archive(struct lar *lar)
+{
+	if (lar == NULL)
+		return;
+
+	if (lar->map != MAP_FAILED)
+		munmap(lar->map, lar->size);
+
+	if (lar->fd >= 0)
+		close(lar->fd);
+
+	free(lar);
+}
+
+/**
+ * Create a new LAR archive - the archive must not exist on disk.
+ * @param archive The filename of the new archive
+ * @param size The intended size of the new archive
+ * @return A LAR archive structure for the new archive
+ */
+struct lar * lar_new_archive(const char *archive, u32 size)
+{
+	struct lar *lar;
+	int i;
+
+	if (!access(archive, F_OK)) {
+		err("Archive file %s already exists\n", archive);
+		return NULL;
+	}
+
+	lar = calloc(sizeof(*lar), 1);
+
+	if (lar == NULL) {
+		err("Unable to allocate memory.\n");
+		return NULL;
+	}
+
+	lar->fd = open(archive, O_RDWR | O_CREAT, S_IRUSR | S_IWUSR);
+
+	if (lar->fd == -1) {
+		err("Couldn't open the archive %s\n", archive);
+
+		free(lar);
+		return NULL;
+	}
+
+	/* Expand the file to the correct size */
+
+	if (lseek(lar->fd, size - 1, SEEK_SET) == -1) {
+		err("Unable to write the archive %s\n", archive);
+		goto err;
+	}
+
+	if (write(lar->fd, "", 1) != 1) {
+		err("Unable to write the file %s\n", archive);
+		goto err;
+	}
+
+	if (_map_lar(lar, size)) {
+		err("Unable to map the archive %s\n", archive);
+		goto err;
+	}
+
+	/* Fill the whole thing with flash friendly 0xFFs */
+	memset(lar->map, 0xFF, lar->size);
+
+	/* Make a dummy bootblock */
+
+	if (lar_add_bootblock(lar, NULL)) {
+		err("Couldn't add a bootblock to %s\n", archive);
+		goto err;
+	}
+
+	return lar;
+ err:
+	lar_close_archive(lar);
+
+	/* Don't leave a halfbaked LAR laying around */
+
+	unlink(archive);
+	return NULL;
+}
+
+/**
+ * Open an existing LAR archive
+ * @param The archive filename to open
+ * @return A LAR archive structure for the opened archive
+ */
+struct lar * lar_open_archive(const char *archive)
+{
+	struct lar *lar;
+	int ret;
+	u32 romlen;
+	struct stat s;
+
+	lar = calloc(sizeof(*lar), 1);
+
+	if (lar == NULL) {
+		err("Unable to allocate memory.\n");
+		return NULL;
+	}
+
+	lar->fd = open(archive, O_RDWR);
+
+	ret = fstat(lar->fd, &s);
+
+	if (ret == -1) {
+		err("Unable to stat %s\n", archive);
+		goto err;
+	}
+
+	if (_map_lar(lar, s.st_size)) {
+		err("Unable to map the archive %s\n", archive);
+		goto err;
+	}
+
+	/* Sanity check - make sure the bootblock header is the same length
+	 * as the LAR archive
+	 */
+
+	romlen = lar_read_size(lar);
+
+	if (romlen != s.st_size) {
+		err("Size mismatch - the header says %d but the file is %d bytes long.\n",
+		    romlen, (int) s.st_size);
+		goto err;
+	}
+
+	return lar;
+
+ err:
+	lar_close_archive(lar);
+	return NULL;
+}
+
+/**
+ * Return the offset of the first chunk of empty space in the LAR
+ * @param lar the LAR archive structure
+ * @return The offset of the first chunk of empty space
+ */
+static int lar_empty_offset(struct lar *lar)
+{
+	u32 offset = 0;
+	struct lar_header *header;
+
+	while (offset < get_bootblock_offset(lar->size)) {
+		header = (struct lar_header *) (lar->map + offset);
+
+		/* We interpet the absence of the magic as empty space */
+
+		if (memcmp(header->magic, MAGIC, 8))
+			break;
+
+		offset += get_next_offset(header);
+	}
+
+	if (offset >= get_bootblock_offset(lar->size))
+		return -1;
+
+	return offset;
+}
+
+/**
+ * Return a 1 if filename is in the list of files - if files is NULL, then
+ * act as if all files are in the list
+ * @param files A list of files to check
+ * @param filename The filename to check against the list
+ * @return 1 if the file is in the list or if the list is NULL, 0 otherwise
+ */
+static int file_in_list(struct file *files, char *filename)
+{
+	struct file *p;
+
+	if (files == NULL)
+		return 1;
+
+	for(p = files ; p; p = p->next) {
+		if (!strcmp(p->name, filename))
+			return 1;
+	}
+
+	return 0;
+}
+
+/**
+ * List the files in a LAR archive
+ * @param lar The LAR archive to list
+ * @param files A list of files to display- if this is NULL, then the default
+ * is to list all files
+ */
+void lar_list_files(struct lar *lar, struct file *files)
+{
+	u8 *ptr = lar->map;
+	char *filename;
+
+	struct lar_header *header;
+	struct file *fp;
+
+	while (ptr < (lar->map + get_bootblock_offset(lar->size))) {
+		header = (struct lar_header *) ptr;
+
+		/* We interpet the absence of the magic as empty space */
+
+		if (strncmp(header->magic, MAGIC, 8))
+			break;
+
+		filename = (char *) (ptr + sizeof(struct lar_header));
+
+		if (file_in_list(files, filename)) {
+			printf("  %s ", filename);
+
+			if (ntohl(header->compression) == none) {
+				printf("(%d bytes @0x%lx)\n",
+				       ntohl(header->len),
+				       (unsigned long)(ptr - lar->map) +
+				       ntohl(header->offset));
+			} else {
+				printf("(%d bytes, %s compressed to %d bytes "
+				       "@0x%lx)\n",
+				       ntohl(header->reallen),
+				       algo_name[ntohl(header->compression)],
+				       ntohl(header->len),
+				       (unsigned long)(ptr - lar->map) +
+				       ntohl(header->offset));
+			}
+		}
+
+		ptr += get_next_offset(header);
+	}
+
+	/* Show the bootblock */
+
+	if (file_in_list(files, BOOTBLOCK_NAME)) {
+		header = (struct lar_header *)
+			(lar->map + get_bootblock_offset(lar->size));
+
+		printf("  bootblock (%d bytes @0x%x)\n",
+		       ntohl(header->len),
+		       get_bootblock_offset(lar->size) +
+		       ntohl(header->offset));
+	}
+}
+
+/**
+ * Write a buffer to a file - this is used to write blobs from a LAR archive
+ * @param filename The file to write
+ * @param buffer The source buffer
+ * @param len The length of the buffer
+ * @return 0 on success , or -1 on failure
+ */
+static int _write_file(char *filename, u8 *buffer, u32 len)
+{
+	char *path = strdup(filename);
+	int fd, ret;
+
+	if (path == NULL) {
+		err("Out of memory.\n");
+		return -1;
+	}
+
+	mkdirp((const char *) dirname(path), 0755);
+	free(path);
+
+	fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0644);
+
+	if (fd == -1) {
+		err("Error creating file %s\n", filename);
+		return -1;
+	}
+
+	ret = write(fd, buffer, len);
+
+	if (ret != len)
+		err("Error writingthe file %s\n", filename);
+
+	close(fd);
+	return (ret == len) ? 0 : -1;
+}
+
+/**
+ * Extract files from a LAR archive.  If files is not NULL, then only
+ * extract the filenames listed within
+ * @param lar The LAR archive to extract from
+ * @param files A list of files to extract.  If NULL then all files are
+ * extracted
+ * @return 0 on success, or -1 on failure
+ */
+int lar_extract_files(struct lar *lar, struct file *files)
+{
+	u8 *ptr = lar->map;
+	char *filename;
+	struct lar_header *header;
+	int ret = 0;
+
+	while (ptr < (lar->map + get_bootblock_offset(lar->size))) {
+
+		header = (struct lar_header *) ptr;
+
+		if (strncmp(header->magic, MAGIC, 8))
+			break;
+
+		filename = (char *) (ptr + sizeof(struct lar_header));
+
+		if (file_in_list(files, filename)) {
+
+			if (ntohl(header->compression) == none) {
+				ret = _write_file(filename,
+						  (u8 *) (ptr + ntohl(header->offset)),
+						  (u32) ntohl(header->len));
+			}
+			else {
+				char *buf =
+					malloc(ntohl(header->reallen));
+
+				if (buf == NULL) {
+					err("Out of memory.\n");
+					return -1;
+				}
+
+				uncompress_functions[ntohl(header->compression)](
+					(char*) buf,
+					(char *) ptr + ntohl(header->offset),
+					ntohl(header->len));
+
+				ret = _write_file(filename, (u8 *) buf,
+						  ntohl(header->reallen));
+
+				free(buf);
+			}
+
+			if (ret == -1)
+				return -1;
+		}
+
+		ptr += get_next_offset(header);
+	}
+
+	if (file_in_list(files, BOOTBLOCK_NAME)) {
+		header = (struct lar_header *)
+			(lar->map + get_bootblock_offset(lar->size));
+
+		ret = _write_file((char *) BOOTBLOCK_NAME,
+				  lar->map + (get_bootblock_offset(lar->size) + ntohl(header->offset)),
+				  BOOTBLOCK_SIZE);
+	}
+
+	return ret;
+}
+
+/**
+ * Add a new file to the LAR archive
+ * @param lar The LAR archive to write into
+ * @param name The name of the file to add
+ * @return  0 on success, or -1 on failure
+ */
+int lar_add_file(struct lar *lar, const char *name)
+{
+	char *filename, *ptr, *temp;
+	enum compalgo thisalgo;
+	struct lar_header *header;
+	u32 offset;
+	int ret, fd, hlen;
+	u32 complen;
+	int pathlen;
+	struct stat s;
+	u32 *walk,  csum;
+
+	/* Find the beginning of the available space in the LAR */
+	offset = lar_empty_offset(lar);
+
+	thisalgo = algo;
+
+	filename = (char *) name;
+
+	if (!strncmp(name, "nocompress:",11)) {
+		filename += 11;
+		thisalgo = none;
+	}
+
+	if (filename[0] == '.' && filename[1] == '/')
+		filename += 2;
+
+	/* Open the file */
+	fd = open(filename, O_RDONLY);
+
+	if (fd == -1) {
+		err("Unable to open %s\n", filename);
+		return -1;
+	}
+
+	if (fstat(fd, &s)) {
+		err("Unable to stat the file %s\n", filename);
+		close(fd);
+		return -1;
+	}
+
+	/* Allocate a temporary buffer to compress into - this is unavoidable,
+	   because we need to make sure that the compressed data will fit in
+	   the LAR, and we won't know the size of the compressed data until
+	   we actually compress it */
+
+	temp = calloc(s.st_size, 1);
+
+	if (temp == NULL) {
+		err("Out of memory.\n");
+		return -1;
+	}
+
+	ptr = mmap(0, s.st_size, PROT_READ, MAP_SHARED, fd, 0);
+
+	if (ptr == MAP_FAILED) {
+		err("Unable to map %s\n", filename);
+		close(fd);
+		free(temp);
+		return -1;
+	}
+
+
+	/* Do the compression step */
+	compress_functions[thisalgo](ptr, s.st_size, temp, &complen);
+
+	if (complen >= s.st_size && (thisalgo != none)) {
+		thisalgo = none;
+		compress_functions[thisalgo](ptr, s.st_size, temp, &complen);
+	}
+
+	munmap(ptr, s.st_size);
+	close(fd);
+
+	pathlen = strlen(filename) + 1 > MAX_PATHLEN ? MAX_PATHLEN : strlen(filename) + 1;
+
+	/* Figure out how big our header will be */
+	hlen = sizeof(struct lar_header) + pathlen;
+	hlen = (hlen + 15) & 0xFFFFFFF0;
+
+	if (offset + hlen + complen >= get_bootblock_offset(lar->size)) {
+		err("Not enough room in the LAR to add the file.\n");
+		free(temp);
+		return -1;
+	}
+
+	/* Lets do this thing */
+
+	/* Zero out the header area */
+	memset(lar->map + offset, 0, hlen);
+
+	header = (struct lar_header *) (lar->map + offset);
+
+	memcpy(header, MAGIC, 8);
+	header->compression = htonl(thisalgo);
+	header->reallen = htonl(s.st_size);
+	header->len = htonl(complen);
+	header->offset = htonl(hlen);
+
+	/* Copy the path name */
+	strncpy((char *) (lar->map + offset + sizeof(struct lar_header)),
+		filename, pathlen - 1);
+
+	/* Copy in the data */
+	memcpy(lar->map + (offset + hlen), temp, complen);
+
+	/* Figure out the checksum */
+
+	csum = 0;
+	for (walk = (u32 *) (lar->map + offset);
+	     walk < (u32 *) (temp + complen + hlen);
+	     walk++) {
+		csum += ntohl(*walk);
+	}
+	header->checksum = htonl(csum);
+
+	free(temp);
+	return 0;
+}
Index: LinuxBIOSv3/util/lar/lib.h
===================================================================
--- LinuxBIOSv3.orig/util/lar/lib.h	2007-07-23 09:46:33.000000000 -0600
+++ LinuxBIOSv3/util/lar/lib.h	2007-07-23 09:47:04.000000000 -0600
@@ -49,6 +49,16 @@
 struct file *get_files(void);
 void free_files(void);
 
+/* Prototypes for the LAR I/O functions */
+struct lar * lar_new_archive(const char *archive, unsigned int size);
+struct lar * lar_open_archive(const char *archive);
+void lar_close_archive(struct lar *lar);
+
+void lar_list_files(struct lar *lar, struct file *files);
+int lar_add_file(struct lar *lar, const char *name);
+int lar_add_bootblock(struct lar *lar, const char *bootblock);
+int lar_extract_files(struct lar *lar, struct file *files);
+
 /* prototypes for extract.c functions */
 int extract_lar(const char *archivename, struct file *files);
 
Index: LinuxBIOSv3/util/lar/lib.c
===================================================================
--- LinuxBIOSv3.orig/util/lar/lib.c	2007-07-23 09:46:33.000000000 -0600
+++ LinuxBIOSv3/util/lar/lib.c	2007-07-23 09:51:11.000000000 -0600
@@ -29,6 +29,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 
+#include "lar.h"
 #include "lib.h"
 
 #define MAX_PATH 1024
@@ -36,6 +37,37 @@
 static struct file *files = NULL;
 
 /**
+ * The default "compress impossible" hook to call when no other compression
+ * is used
+ */
+void compress_impossible(char *in, u32 in_len, char *out, u32 *out_len)
+{
+	fprintf(stderr,
+		"The selected compression algorithm wasn't compiled in.\n");
+	exit(1);
+}
+
+/**
+ * The default "compress" hook to call when no other compression is used
+ */
+void do_no_compress(char *in, u32 in_len, char *out, u32 *out_len)
+{
+	memcpy(out, in, in_len);
+	out_len[0] = in_len;
+}
+
+/**
+ * The default "uncompress" hook to call when no other compression is used
+ */
+void uncompress_impossible(char *dst, char *src, u32 len)
+{
+	fprintf(stderr,
+		"Cannot uncompress data (algorithm not compiled in).\n");
+	exit(1);
+}
+
+
+/**
  * Create a new directory including any missing parent directories.
  *
  * NOTE: This function does not do complete path resolution as described in

--
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 2/5] New LAR access functions

[Peter Stuge]

On Mon, Jul 23, 2007 at 10:06:04AM -0600, [e-mail] wrote:
> In preparation for adding new LAR functionality - this patch
> consolidates creating and accessing the LAR into new code utilizing
> mmap which facilitates moving about within the archive.

This part is fine..
> This code also turns the bootblock path name as a constant value.

..but I think this is a disaster. I don't think lar should know about
bootblocks, it should instead offer the functionality needed for the
caller to put special files in special places within the larfile.
> It also requires that the user specify a size when the LAR is
> created.

Since the larfile is supposed to be 1:1 of the flash chip I think
this requirement is unavoidable.


//Peter

[LinuxBIOS] New LAR access functions

[Jordan Crouse]

On 25/07/07 20:36 +0200, Peter Stuge wrote:
> > This code also turns the bootblock path name as a constant value.
>
> ..but I think this is a disaster. I don't think lar should know about
> bootblocks, it should instead offer the functionality needed for the
> caller to put special files in special places within the larfile.

Fair enough.  I've been thinking about something like that, being able
to control where in the LAR the blob goes (as well as being able to control
where in memory it gets loaded to).

But seeing as though the bootblock is 1) very special, and 2) mandatory
for the LAR, I don't think there is any problem in calling it out specially,
especially since the position and the concept is a hard constant (for now).

Otherwise, the alternative is for the user to have to do the math, and know
that they should put the bootblock at size - 16k in the LAR.  That seems
problematic.
> > It also requires that the user specify a size when the LAR is
> > created.
>
> Since the larfile is supposed to be 1:1 of the flash chip I think
> this requirement is unavoidable.

Yes, but it further breaks the 'ar' / 'tar' / 'lar' similarity.

Jordan
-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] New LAR access functions

[Stefan Reinauer]

* Jordan Crouse <[e-mail]> [070725 21:35]:
> Fair enough.  I've been thinking about something like that, being able
> to control where in the LAR the blob goes (as well as being able to control
> where in memory it gets loaded to).
>
> But seeing as though the bootblock is 1) very special, and 2) mandatory
> for the LAR, I don't think there is any problem in calling it out specially,
> especially since the position and the concept is a hard constant (for now).

I had an sc520 board with 16 mbit flash. the bios would sit in the
middle of it, from 6 mbit to 8 mbit. Sounds like fun, hm?
> Otherwise, the alternative is for the user to have to do the math, and know
> that they should put the bootblock at size - 16k in the LAR.  That seems
> problematic.

Yes, this is totally contrary to making things easier for the user.
> > > It also requires that the user specify a size when the LAR is
> > > created.
> >
> > Since the larfile is supposed to be 1:1 of the flash chip I think
> > this requirement is unavoidable.
>
> Yes, but it further breaks the 'ar' / 'tar' / 'lar' similarity.

I dont think that similarity is worth much. Let's design a clean utility
that does the job it does. If it is easy to use because it people
recognize the way it is used, that's fine. But that is not the main
reason. The main reason it should be easy to use is that it knows how to
do the one job really well and without tunables that never get tuned.

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[LinuxBIOS] [patch 2/5] New LAR access functions

[Stefan Reinauer]

* Peter Stuge <[e-mail]> [070725 20:36]:
> > This code also turns the bootblock path name as a constant value.
>
> ..but I think this is a disaster. I don't think lar should know about
> bootblocks, it should instead offer the functionality needed for the
> caller to put special files in special places within the larfile.

It already knows about bootblocks, and that is an absolute requirement.

There is a lot more to it that putting files in certain places. The
bootblock needs to be patched up so that linuxbios knows how large the
lar file is, and potentially lots of other things.

Before we have a good concept on how to layout those files and who is
going to do that, we should not try to make things generic in a way that
we don't even plan to use.

Let's try to get this working and then enhance it to become funky.

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[LinuxBIOS] [patch 2/5] New LAR access functions

[Peter Stuge]

On Mon, Jul 23, 2007 at 10:06:04AM -0600, [e-mail] wrote:
> + * Write a buffer to a file - this is used to write blobs from a
> LAR archive

How are blobs different from other files in larfiles?
> +	mkdirp((const char *) dirname(path), 0755);

Please use mkdirp_below() and pass "." for the parent parameter.


//Peter

[LinuxBIOS] [patch 2/5] New LAR access functions

[Stefan Reinauer]

* Peter Stuge <[e-mail]> [070725 20:38]:
> > +	mkdirp((const char *) dirname(path), 0755);
>
> Please use mkdirp_below() and pass "." for the parent parameter.

We should include the "." in the function if we never call it with any
other directory.

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[LinuxBIOS] [patch 2/5] New LAR access functions

[Peter Stuge]

On Wed, Jul 25, 2007 at 09:56:11PM +0200, Stefan Reinauer wrote:
> * Peter Stuge <[e-mail]> [070725 20:38]:
> > Please use mkdirp_below() and pass "." for the parent parameter.
>
> We should include the "." in the function if we never call it with
> any other directory.

Sure.

I made it a parameter because it is good for when lar gets a -C (ie.
extract into that directory over there) but we can change back
if/when that happens.


//Peter

[LinuxBIOS] [patch 2/5] New LAR access functions

[ron minnich]

Guys, this patch appears to be stuck in limbo. It is holding up the GX port.

Did we resolve this or did it just sort of die? Can we get it worked
out? We need to jumpstart things again.

thanks

ron

On 7/25/07, Peter Stuge <[e-mail]> wrote:
> On Wed, Jul 25, 2007 at 09:56:11PM +0200, Stefan Reinauer wrote:
> > * Peter Stuge <[e-mail]> [070725 20:38]:
> > > Please use mkdirp_below() and pass "." for the parent parameter.
> >
> > We should include the "." in the function if we never call it with
> > any other directory.
>
> Sure.
>
> I made it a parameter because it is good for when lar gets a -C (ie.
> extract into that directory over there) but we can change back
> if/when that happens.
>
> //Peter

[LinuxBIOS] [patch 2/5] New LAR access functions

[Stefan Reinauer]

* ron minnich <[e-mail]> [070807 06:47]:
> Guys, this patch appears to be stuck in limbo. It is holding up the GX port.
>
> Did we resolve this or did it just sort of die? Can we get it worked
> out? We need to jumpstart things again.

go ahead and we fix it later if its broken.

Feel free to beat me afterwards for it, but we need some progress:
Acked-by: Stefan Reinauer <[e-mail]>


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[LinuxBIOS] [patch 2/5] New LAR access functions

[Stefan Reinauer]

[e-mail] wrote:
> In preparation for adding new LAR functionality - this patch consolidates
> creating and accessing the LAR into new code utilizing mmap which
> facilitates moving about within the archive.
>
> This code also turns the bootblock path name as a constant value.
> It also requires that the user specify a size when the LAR is
> created.
>
> Signed-off-by: Jordan crouse <[e-mail]>

This code checks whether the target file already exists and fails if this is the case. 

I am not sure whether this is an intended behavior. When running "make" two times in a row, the second make fails now:
> Archive file ../linuxbios.rom already exists Unable to create
> ../linuxbios.rom as a LAR archive. make: ***
> [/home/stepan/svn/LinuxBIOSv3/build/linuxbios.rom] Error 1
> I fixed this in the Makefile for now by deleting the output file before creating it.

[LinuxBIOS] [patch 3/5] Allow user to specify pathnames for create and add

Add another field to the filename specified for create and add operations
to specify the intended pathname for the blob.

Signed-off-by: Jordan Crouse <[e-mail]>
Index: LinuxBIOSv3/util/lar/stream.c
===================================================================
--- LinuxBIOSv3.orig/util/lar/stream.c	2007-07-23 09:47:04.000000000 -0600
+++ LinuxBIOSv3/util/lar/stream.c	2007-07-23 09:51:39.000000000 -0600
@@ -543,6 +543,8 @@
 int lar_add_file(struct lar *lar, const char *name)
 {
 	char *filename, *ptr, *temp;
+	char *pathname;
+
 	enum compalgo thisalgo;
 	struct lar_header *header;
 	u32 offset;
@@ -567,6 +569,20 @@
 	if (filename[0] == '.' && filename[1] == '/')
 		filename += 2;
 
+	pathname = strchr(filename, ':');
+
+	if (pathname != NULL) {
+	  *pathname = '\0';
+	  pathname++;
+
+	  if (!strlen(pathname)) {
+	    err("Invalid pathname specified.\n");
+	    return -1;
+	  }
+	}
+	else
+		pathname = filename;
+
 	/* Open the file */
 	fd = open(filename, O_RDONLY);
 
@@ -614,7 +630,7 @@
 	munmap(ptr, s.st_size);
 	close(fd);
 
-	pathlen = strlen(filename) + 1 > MAX_PATHLEN ? MAX_PATHLEN : strlen(filename) + 1;
+	pathlen = strlen(pathname) + 1 > MAX_PATHLEN ? MAX_PATHLEN : strlen(pathname) + 1;
 
 	/* Figure out how big our header will be */
 	hlen = sizeof(struct lar_header) + pathlen;
@@ -641,7 +657,7 @@
 
 	/* Copy the path name */
 	strncpy((char *) (lar->map + offset + sizeof(struct lar_header)),
-		filename, pathlen - 1);
+		pathname, pathlen - 1);
 
 	/* Copy in the data */
 	memcpy(lar->map + (offset + hlen), temp, complen);
Index: LinuxBIOSv3/util/lar/lib.c
===================================================================
--- LinuxBIOSv3.orig/util/lar/lib.c	2007-07-23 09:51:11.000000000 -0600
+++ LinuxBIOSv3/util/lar/lib.c	2007-07-23 09:51:39.000000000 -0600
@@ -195,16 +195,29 @@
 {
 	struct stat filestat;
 	int ret = -1;
-	const char *realname;
+	char *realname;
+	char *c;
 
-	realname = name;
 	if (strstr(name, "nocompress:") == name) {
-		realname = name + 11;
+		name += 11;
 	}
 
+	realname = strdup(name);
+
+	if (realname == NULL) {
+	  fprintf(stderr, "Out of memory.\n");
+	  exit(1);
+	}
+
+	c = strchr(realname, ':');
+
+	if (c != NULL)
+	  *c = '\0';
+
 	/* printf("... add_files %s\n", name); */
 	if (stat(realname, &filestat) == -1) {
 		fprintf(stderr, "Error getting file attributes of %s\n", name);
+		free(realname);
 		return -1;
 	}
 
@@ -249,6 +262,7 @@
 		ret = 0;
 	}
 
+	free(realname);
 	return ret;
 }
 

--
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 3/5] Allow user to specify pathnames for create and add

[Carl-Daniel Hailfinger]

On 23.07.2007 18:06, [e-mail] wrote:
> Add another field to the filename specified for create and add operations
> to specify the intended pathname for the blob.
>
> Signed-off-by: Jordan Crouse <[e-mail]>
> Index: LinuxBIOSv3/util/lar/stream.c
> ===================================================================
> --- LinuxBIOSv3.orig/util/lar/stream.c	2007-07-23 09:47:04.000000000 -0600
> +++ LinuxBIOSv3/util/lar/stream.c	2007-07-23 09:51:39.000000000 -0600
> @@ -567,6 +569,20 @@
>   if (filename[0] == '.' && filename[1] == '/')
>       filename += 2;
>
> +	pathname = strchr(filename, ':');
> +
> +	if (pathname != NULL) {
> +	  *pathname = '\0';
> +	  pathname++;
> +
> +	  if (!strlen(pathname)) {
> +	    err("Invalid pathname specified.\n");
> +	    return -1;
> +	  }
> +	}

We might want to add a notice to the man page/usage info that filenames
with a colon are invalid.

Regards
Carl-Daniel

[LinuxBIOS] [patch 3/5] Allow user to specify pathnames for create and add

[Stefan Reinauer]

[e-mail] wrote:
> Add another field to the filename specified for create and add operations
> to specify the intended pathname for the blob.
>
> Signed-off-by: Jordan Crouse <[e-mail]>
> This patch breaks the produced image. Please provide a fix.

[LinuxBIOS] Allow user to specify pathnames for create and add

[Jordan Crouse]

On 20/08/07 02:01 +0200, Stefan Reinauer wrote:
> [e-mail] wrote:
> > Add another field to the filename specified for create and add operations
> > to specify the intended pathname for the blob.
> >
> > Signed-off-by: Jordan Crouse <[e-mail]>
> > This patch breaks the produced image. Please provide a fix.

Not sure what you mean - but this code has suffered from too much bit rot,
so I'm going to withdraw all of them.  Better luck next time, I guess. :)

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] Allow user to specify pathnames for create and add

[Stefan Reinauer]

Dear Jordan,

* Jordan Crouse <[e-mail]> [070820 16:19]:
> On 20/08/07 02:01 +0200, Stefan Reinauer wrote:
> > [e-mail] wrote:
> > > Add another field to the filename specified for create and add operations
> > > to specify the intended pathname for the blob.
> > >
> > > Signed-off-by: Jordan Crouse <[e-mail]>
> > > This patch breaks the produced image. Please provide a fix.
>
> Not sure what you mean - but this code has suffered from too much bit rot,
> so I'm going to withdraw all of them.  Better luck next time, I guess. :)

No, it seems it's not a problem of bit rot, that code has not been
touched in the repository between the time you made the changes and the
time it got checked in. It just seems to be a corner case that is not
fully working yet with the new code base. I narrowed the problem down to
the util/lar/lib.c part of the patch which causes all modules to be
compressed, even those with a nocompress: tag.

It's too late for a withdraw now since your fine piece of work is
already in the v3 repository since yesterday night ;-) Please don't ask
me to remove it again. I believe this is a significant step forward on
our road to v3.

I want to apologize for the long delay since you provided the code - The
final version of the patches have been sitting in my inbox and on my
todo list since July 23rd. Time has been a very tight resource for me,
just like for many other core LinuxBIOS developers lately. We're trying
as good as we can to change that.

Let's try to get things going again so we don't lose the momentum we had
in Ottawa!

Best wishes,

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/

[LinuxBIOS] Allow user to specify pathnames for create and add

[Patrick Georgi]

Stefan Reinauer wrote:
> fully working yet with the new code base. I narrowed the problem down to
> the util/lar/lib.c part of the patch which causes all modules to be
> compressed, even those with a nocompress: tag.
> the attached patch should fix it..

Regards,
Patrick Georgi

[LinuxBIOS] [patch 4/5] Add an "ADD" mode to the LAR utility

Provide the ability to add arbitrary files to an existing LAR.  Also
fleshed out the "usage" screen.

Signed-off-by: Jordan Crouse <[e-mail]>
Index: LinuxBIOSv3/util/lar/lar.c
===================================================================
--- LinuxBIOSv3.orig/util/lar/lar.c	2007-07-11 11:41:37.000000000 -0600
+++ LinuxBIOSv3/util/lar/lar.c	2007-07-11 11:51:57.000000000 -0600
@@ -44,7 +44,33 @@
 static void usage(char *name)
 {
 	printf("\nLAR - the LinuxBIOS Archiver " VERSION "\n" COPYRIGHT "\n\n"
-	       "Usage: %s [-cxl] archive.lar [[[file1] file2] ...]\n\n", name);
+	       "Usage: %s [-cxal] archive.lar [[[file1] file2] ...]\n\n", name);
+	printf("Examples:\n");
+	printf("  lar -c -s 32768 -b bootblock myrom.lar foo nocompress:bar\n");
+	printf("  lar -a myrom.lar foo blob:baz\n");
+	printf("  lar -l myrom.lar\n\n");
+
+	printf("File names:\n");
+	printf("   Names specified in the create or add modes are formatted as\n");
+	printf("   follows:  [flags]:[filename]:[pathname].\n");
+	printf("     * Flags are modifiers for the file.  Valid flags:\n");
+	printf("       nocompress\tDon't compress the file in the LAR\n");
+	printf("     * Filename is the name of the file on disk.  If no pathname\n");
+	printf("     is specified, then the filename will also be the path name\n");
+	printf("     used in the LAR.\n");
+	printf("     * Pathname is the name to use in the LAR header.\n\n");
+
+	printf("Create options:\n");
+	printf("  -s [size]\tSpecify the size of the archive (in bytes)\n");
+	printf("  -b [bootblock]\tSpecify the bootblock blob\n");
+	printf("  -C [lzma|nrv2b]\tSpecify the compression method to use\n\n");
+
+	printf("General options\n");
+	printf("  -v\tEnable verbose mode\n");
+	printf("  -V\tShow the version\n");
+	printf("  -h\tShow this help\n");
+	printf("\n");
+
 }
 
 int verbose(void)
@@ -90,6 +116,27 @@
 	return 0;
 }
 
+int add_lar(const char *archivename, struct file *files)
+{
+	struct lar *lar = lar_open_archive(archivename);
+
+	if (lar == NULL) {
+		fprintf(stderr, "Unable to open LAR archive %s\n", archivename);
+		exit(1);
+	}
+
+	for( ; files; files = files->next) {
+		if (lar_add_file(lar, files->name)) {
+			fprintf(stderr, "Error adding %s to the LAR.\n", files->name);
+			lar_close_archive(lar);
+			exit(1);
+		}
+	}
+
+	lar_close_archive(lar);
+	return 0;
+}
+
 int list_lar(const char *archivename, struct file *files)
 {
 	struct lar *lar = lar_open_archive(archivename);
@@ -130,6 +177,7 @@
 	char *archivename = NULL;
 
 	static struct option long_options[] = {
+		{"add", 0, 0, 'a'},
 		{"create", 0, 0, 'c'},
 		{"compress-algo", 1, 0, 'C'},
 		{"extract", 0, 0, 'x'},
@@ -147,9 +195,12 @@
 		exit(1);
 	}
 
-	while ((opt = getopt_long(argc, argv, "cC:xls:b:vVh?",
+	while ((opt = getopt_long(argc, argv, "acC:xls:b:vVh?",
 				  long_options, &option_index)) != EOF) {
 		switch (opt) {
+		case 'a':
+			larmode  = ADD;
+			break;
 		case 'c':
 			larmode = CREATE;
 			break;
@@ -258,6 +309,9 @@
 	}
 
 	switch (larmode) {
+	case ADD:
+		add_lar(archivename, get_files());
+		break;
 	case EXTRACT:
 		extract_lar(archivename, get_files());
 		break;
Index: LinuxBIOSv3/util/lar/lib.h
===================================================================
--- LinuxBIOSv3.orig/util/lar/lib.h	2007-07-11 11:48:27.000000000 -0600
+++ LinuxBIOSv3/util/lar/lib.h	2007-07-11 11:50:40.000000000 -0600
@@ -30,6 +30,7 @@
 
 enum {
 	NONE,
+	ADD,
 	CREATE,
 	LIST,
 	EXTRACT

--
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 4/5] Add an "ADD" mode to the LAR utility

[Carl-Daniel Hailfinger]

On 23.07.2007 18:06, [e-mail] wrote:
> Provide the ability to add arbitrary files to an existing LAR.  Also
> fleshed out the "usage" screen.
>
> Signed-off-by: Jordan Crouse <[e-mail]>
> Index: LinuxBIOSv3/util/lar/lar.c
> ===================================================================
> --- LinuxBIOSv3.orig/util/lar/lar.c	2007-07-11 11:41:37.000000000 -0600
> +++ LinuxBIOSv3/util/lar/lar.c	2007-07-11 11:51:57.000000000 -0600
> @@ -44,7 +44,33 @@
>  static void usage(char *name)
>  {
>   printf("\nLAR - the LinuxBIOS Archiver " VERSION "\n" COPYRIGHT "\n\n"
> -	       "Usage: %s [-cxl] archive.lar [[[file1] file2] ...]\n\n", name);
> +	       "Usage: %s [-cxal] archive.lar [[[file1] file2] ...]\n\n", name);
> +	printf("Examples:\n");
> +	printf("  lar -c -s 32768 -b bootblock myrom.lar foo nocompress:bar\n");
> +	printf("  lar -a myrom.lar foo blob:baz\n");

"blob" flag used here
> +	printf("  lar -l myrom.lar\n\n");
> +
> +	printf("File names:\n");
> +	printf("   Names specified in the create or add modes are formatted as\n");
> +	printf("   follows:  [flags]:[filename]:[pathname].\n");
> +	printf("     * Flags are modifiers for the file.  Valid flags:\n");
> +	printf("       nocompress\tDon't compress the file in the LAR\n");

"blob" flag not mentioned here
> +	printf("     * Filename is the name of the file on disk.  If no pathname\n");

Regards,
Carl-Daniel

[LinuxBIOS] [patch 4/5] Add an "ADD" mode to the LAR utility

[Peter Stuge]

This patch aside, I would like a better way to provide the flags and
pathname rather than around the filename, but I can't think of
anything really simple. The manifest file is the best idea so far,
but I would like to be able to run lar without one too.


On Mon, Jul 23, 2007 at 10:06:06AM -0600, [e-mail] wrote:
> +		case 'a':
> +			larmode  = ADD;
> +			break;

Extra space before =, please fix that before commit.

Acked-by: Peter Stuge <[e-mail]>

[LinuxBIOS] Add an "ADD" mode to the LAR utility

[Jordan Crouse]

On 25/07/07 20:29 +0200, Peter Stuge wrote:
> This patch aside, I would like a better way to provide the flags and
> pathname rather than around the filename, but I can't think of
> anything really simple. The manifest file is the best idea so far,
> but I would like to be able to run lar without one too.

Agreed.  I thought about this for a while myself, and this is the best
I've come up with (thats not saying much, though), that fit in the
current model.

Another option might be to use an 'expression' based command line, with
options processed in order in the command line (like 'find' does):

./lar --add --file foo --path normal/foo --nocompress \
	--file bar --path fallback/bar

where --path and --nocompress are modifiers for the blob previously 
specified with --file.

Another option similar to that would be to only allow a single file to
be specified, and only during the 'add' function.

./lar --create --bootblock bootblock --size 16k <-- create the LAR
./lar --add --file stage1 --path normal/stage1
./lar --add --file stage2 --path normal/stage2 --nocompress
...

and so on and so forth.

Neither of these options seem very clever to me, so I am completely
open to suggestions.

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 5/5] Allow human friendly suffixes for the size option

Enable 'k' and 'm' suffixes to modify the size in a human friendly
way - s 32k is much easier on the brain then -s 32768.

Signed-off-by: Jordan Crouse <[e-mail]>
Index: LinuxBIOSv3/util/lar/lar.c
===================================================================
--- LinuxBIOSv3.orig/util/lar/lar.c	2007-07-11 17:40:40.000000000 -0600
+++ LinuxBIOSv3/util/lar/lar.c	2007-07-11 17:41:12.000000000 -0600
@@ -46,7 +46,7 @@
 	printf("\nLAR - the LinuxBIOS Archiver " VERSION "\n" COPYRIGHT "\n\n"
 	       "Usage: %s [-cxal] archive.lar [[[file1] file2] ...]\n\n", name);
 	printf("Examples:\n");
-	printf("  lar -c -s 32768 -b bootblock myrom.lar foo nocompress:bar\n");
+	printf("  lar -c -s 32k -b bootblock myrom.lar foo nocompress:bar\n");
 	printf("  lar -a myrom.lar foo blob:baz\n");
 	printf("  lar -l myrom.lar\n\n");
 
@@ -61,7 +61,9 @@
 	printf("     * Pathname is the name to use in the LAR header.\n\n");
 
 	printf("Create options:\n");
-	printf("  -s [size]\tSpecify the size of the archive (in bytes)\n");
+	printf("  -s [size]     \tSpecify the size of the archive.\n");
+	printf("                \tUse a 'k' suffix to multiply the size by 1024 or\n");
+	printf("                \ta 'm' suffix to multiple the size by 1024*1024.\n");
 	printf("  -b [bootblock]\tSpecify the bootblock blob\n");
 	printf("  -C [lzma|nrv2b]\tSpecify the compression method to use\n\n");
 
@@ -73,6 +75,29 @@
 
 }
 
+/* Add a human touch to the LAR size by allowing suffixes:
+ *  XX[KkMm] where k = XX * 1024 and m or M = xx * 1024 * 1024
+ */
+
+static void parse_larsize(char *str)
+{
+	char *p = NULL;
+	unsigned int size = strtoul(str, &p, 0);
+
+	if (p != NULL) {
+		if (*p == 'k' || *p == 'K')
+			size *= 1024;
+		else if (*p == 'm' || *p == 'M')
+			size *= (1024 * 1024);
+		else {
+			fprintf(stderr, "Unknown LAR size suffix %c\n", *p);
+			exit(1);
+		}
+	}
+
+	larsize = size;
+}
+
 int verbose(void)
 {
 	return isverbose;
@@ -219,7 +244,7 @@
 			larmode = EXTRACT;
 			break;
 		case 's':
-			larsize = strtol(optarg, (char **)NULL, 10);
+			parse_larsize(optarg);
 			break;
 		case 'b':
 			bootblock = strdup(optarg);

--
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] [patch 5/5] Allow human friendly suffixes for the size option

[Peter Stuge]

On Mon, Jul 23, 2007 at 10:06:07AM -0600, [e-mail] wrote:
> +static void parse_larsize(char *str)
> +{
> +	char *p = NULL;
> +	unsigned int size = strtoul(str, &p, 0);

larsize is declared a long while parse_larsize() now uses strtoul() -
they should match.
> case 's':
> -			larsize = strtol(optarg, (char **)NULL, 10);
> +			parse_larsize(optarg);

Another issue was already discussed;

Either make parse_larsize() return a value, or put the code in the
case.

File global larsize is yucky and hurts reuse.


//Peter

[LinuxBIOS] Allow human friendly suffixes for the size option

[Jordan Crouse]

On 23/07/07 19:37 +0200, Peter Stuge wrote:
> File global larsize is yucky and hurts reuse.

Updated patch removing global larsize is attached.  

Now, not to be a jerk, but if there are no more comments, can I get some
ACKs, so I can get this code in? :-)

Jordan

-- 
Jordan Crouse
Systems Software Development Engineer 
Advanced Micro Devices, Inc.

[LinuxBIOS] Allow human friendly suffixes for the size option

[Peter Stuge]

On Wed, Jul 25, 2007 at 09:34:44AM -0600, Jordan Crouse wrote:
> [PATCH][LAR]  Allow human friendly suffixes for the size option
>
> Enable 'k' and 'm' suffixes to modify the size in a human friendly
> way - s 32k is much easier on the brain then -s 32768.
>
> Signed-off-by: Jordan Crouse <[e-mail]>

Acked-by: Peter Stuge <[e-mail]>

[LinuxBIOS] [patch 5/5] Allow human friendly suffixes for the size option

[Stefan Reinauer]

[e-mail] wrote:
> Enable 'k' and 'm' suffixes to modify the size in a human friendly
> way - s 32k is much easier on the brain then -s 32768.
>
> Signed-off-by: Jordan Crouse <[e-mail]>

This patch breaks the old behavior of being able to specify the number of bytes without a unit. 

I'll commit it with a fix.


-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: [phone] • Fax: [phone]
Email: [e-mail]  • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866