[U-Boot] [PATCH v3] Add imls utility command

[Wolfgang Denk]

Dear Marco,

In message <49D8E8F3.4010300 at gmail.com> you wrote:
> 
> >> +	for (j = sectoroffset; j < sectorcount; ++j) {
> > 
> > i. e. add
> > 
> > 		if (lseek(fd, j*sectorsize, SEEK_SET) != j*sectorsize) {
> > 			error handling goes here
> > 		}
> 
> Mmm...I don't well understand why. For example if a sector is 128k and
> the header size is for example of 100 byte, after the read below, at the
> beginning of the cycle, the offset will be 100 byte (we say that
> sectoroffset is 0) and for example we don't find any image. At next
> cycle if I do an lseek of 1*128K, I'll get (1*128K)+100 byte as offset,
> at next (2*128k) + 200 byte and so on. To do this I should do an lseek
> after the read as lseek(fd,-readbyte,SEEK_CUR).

Please think about this again. We use a single lseek() before each
read() to position the read pointer at the next sector boundary, i.e.
where we need it. We don't care whe reit is after the read().

...
> >> +	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.
> I can't know in this case the size of the data, to remove the malloc I
> have to allocate a fixed buffer of X byte, I think in this case a malloc
> is better.

No. Who claims you must read the whole image at once? Use a fixed
buffer of a convenient size (say PAGE_SIZE), and read the data
sequentially. No need to allocate zillions of megabytes of memory at
once.

> >> +	lseek(fd, -len, SEEK_CUR);
> > 
> > Why would this lseek() be needed? [And ifit ws needed, you should
> > check it's return value.]
> 
> It was needed to be aligned to sector boundary and set the file offset
> as before to call this function (see my comment above).

Your thinking is inverted. Don't try to fix the read pointer after
each read(). Set it as needed before the read() when necessary.

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: wd at denx.de
A memorandum is written not to inform the reader, but to protect  the
writer.                                               -- Dean Acheson