Dear Marco,
In message <49d74e7b....@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
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Swap read error. You lose your mind.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
