Hi Wolfgang,
2009/4/5 Wolfgang Denk <w...@denx.de>:
> Dear Marco,
>
> In message <49d8e8f3.4010...@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: w...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
