
Dear Marco,
In message 49D74E7B.904@gmail.com you wrote:
Fixed a memory leak in image_verify_header function. Used the functions image_get_xxxx() from image.h. Fixed two sector boundary misalignment while reading.
...
- err = ioctl(fd, MEMGETINFO, &mtdinfo);
- if (err < 0) {
fprintf(stderr, "%s: Cannot get MTD information: %s\n",cmdname,strerror(errno));
Line too long (check also rest of file).
exit(EXIT_FAILURE);
- }
You probably want to check for unsupported flash types, too - see example in "tools/env/fw_env.c"
- mtdsize = mtdinfo.size;
- if (sectorsize * sectorcount != mtdsize) {
fprintf(stderr, "%s: Partition size (%d) incompatible with sector size and count\n", cmdname, mtdsize);
exit(EXIT_FAILURE);
- }
Hmm... what happens if an image starts in one of the small sectors in a bottom boot type flash?
...
- buf = malloc(sizeof(char)*dim);
Why not just use a dynamic array on the stack? Get rid of all this malloc() and free() stuff...
- if (!buf) {
fprintf (stderr, "%s: Can't allocate memory: %s\n",
cmdname, strerror(errno));
exit(EXIT_FAILURE);
- }
- if (sectoroffset != 0)
lseek(fd, sectoroffset*sectorsize, SEEK_SET);
Delete these two lines and mode them into the loop...
- printf("Searching....\n");
- for (j = sectoroffset; j < sectorcount; ++j) {
i. e. add
if (lseek(fd, j*sectorsize, SEEK_SET) != j*sectorsize) { error handling goes here }
readbyte = read(fd, buf, dim);
if (readbyte != dim) {
fprintf(stderr, "%s: Can't read from device: %s\n",
cmdname, strerror(errno));
exit(EXIT_FAILURE);
}
if (fdt_check_header(buf)) {
/* old-style image */
if (image_verify_header(buf, fd)) {
found = 1;
image_print_contents((image_header_t *)buf);
}
} else {
/* FIT image */
fit_print_contents(buf);
}
lseek(fd, diff, SEEK_CUR); /* Align to next sector boundary */
Delete this line then, too.
+void usage() +{
- fprintf (stderr, "Usage:\n"
" %s [-o offset] -s count -c size \n"
" -o ==> number of sectors to use as offset\n"
" -s ==> number of sectors\n"
" -c ==> size of sectors\n",
cmdname);
Usage message is not correct - the device argument is missing.
- exit (EXIT_FAILURE);
+}
+static int image_verify_header(char *ptr, int fd) +{
- int len, n;
- char *data;
- uint32_t checksum;
- image_header_t *hdr = (image_header_t *)ptr;
- if (image_get_magic(hdr) != IH_MAGIC) {
return 0;
- }
- data = (char *)hdr;
- len = image_get_header_size();
- checksum = image_get_hcrc(hdr);
- hdr->ih_hcrc = htonl(0); /* clear for re-calculation */
- if (crc32(0, data, len) != checksum) {
fprintf (stderr,
"%s: Maybe image found but it has bad header checksum!\n",
cmdname);
return 0;
- }
- len = image_get_size(hdr);
- data = malloc(len * sizeof(char));
Come on. sizeof(char) ? You must be joking.
- if (!data) {
fprintf (stderr, "%s: Can't allocate memory: %s\n",
cmdname, strerror(errno));
exit(EXIT_FAILURE);
- }
Get rid of this malloc(), too.
- n = read(fd, data, len);
- if (n != len) {
fprintf (stderr,
"%s: Error while reading: %s\n",
cmdname, strerror(errno));
exit(EXIT_FAILURE);
- }
- lseek(fd, -len, SEEK_CUR);
Why would this lseek() be needed? [And ifit ws needed, you should check it's return value.]
diff -uprN u-boot-2009.03-orig/tools/imls.h u-boot-2009.03/tools/imls.h --- u-boot-2009.03-orig/tools/imls.h 1970-01-01 01:00:00.000000000 +0100 +++ u-boot-2009.03/tools/imls.h 2009-03-29 11:43:31.000000000 +0200 @@ -0,0 +1,41 @@
...
+#include <errno.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <stddef.h> +#include <string.h> +#include <sys/types.h> +#include <sys/ioctl.h> +#include <sys/stat.h> +#include <unistd.h>
+#ifdef MTD_OLD +# include <stdint.h> +# include <linux/mtd/mtd.h> +#else +# define __user /* nothing */ +# include <mtd/mtd-user.h> +#endif
+#include <sha1.h> +#include "fdt_host.h" +#include <image.h>
Such a header file makes no sense, especially with just a single user. Dump it.
diff -uprN u-boot-2009.03-orig/tools/Makefile u-boot-2009.03/tools/Makefile --- u-boot-2009.03-orig/tools/Makefile 2009-03-21 22:04:41.000000000 +0100 +++ u-boot-2009.03/tools/Makefile 2009-03-28 18:47:38.000000000 +0100
Please also rebase against current top of tree.
Best regards,
Wolfgang Denk