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

[Marco Stornelli]

Hi Wolfgang,

2009/4/5 Wolfgang Denk <wd at denx.de>:
> 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().
>

Oops sorry, I've read SEEK_CUR instead of SEEK_SET, ok in this way works.

> ...
>> >> +  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.
>

Ok, I'll try to do that sequentially.

>> >> +  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
>

I'll send a new patch with the modification as soon as