[PATCH 2/2] dfu_load.c: Do not download file suffix

Tormod Volden lists.tormod at gmail.com
Tue May 24 23:10:41 CEST 2011


This also missed the list. Patch version 2 will follow.

Tormod

On Tue, May 24, 2011 at 8:45 AM, Stefan Schmidt wrote:
> Hello.
>
> On Mon, 2011-05-23 at 23:35, Tormod Volden wrote:
>> On Mon, May 23, 2011 at 7:46 PM, Stefan Schmidt wrote:
>> >> -     while (bytes_sent < st.st_size /* - DFU_HDR */) {
>> >> +     while (bytes_sent < file.size - file.suffixlen) {
>> >>               int hashes_todo;
>> >> +             int chunk_size;
>> >>
>> >> -             ret = read(fd, buf, xfer_size);
>> >> +             chunk_size = file.size - file.suffixlen - bytes_sent;
>> >> +             if (chunk_size > xfer_size)
>> >> +                     chunk_size = xfer_size;
>> >> +             ret = read(file.fd, buf, chunk_size);
>> >
>> > The rest are pretty obvious changes due to the new struct but this
>> > jumped in my eye.
>> >
>> > Did you hit an actual bug with a to large xfer_size?
>>
>> No, it is just obfuscated code :) I should maybe have used other variable names.
>> The normal chunk size is the xfer_size. The only exception is if the
>> last part left is smaller than xfer_size, then use this smaller size.
>>
>> This is easier to understand:
>> >> +             bytes_left = file.size - file.suffixlen - bytes_sent;
>> >> +             if (bytes_left <  xfer_size)
>> >> +                     chunk_size = bytes_left;
>> >> +             else
>> >> +                     chunk_size = xfer_size;
>> >> +             ret = read(file.fd, buf, chunk_size);
>
> I like this one most. Good variable names and easy to follow the
> intention.
>
>> But it was longer to type :) Unless I use MIN macro or a conditional expression:
>> >> +             bytes_left = file.size - file.suffixlen - bytes_sent;
>> >> +             ret = read(file.fd, buf, bytes_left <  xfer_size ? bytes_left : xfer_size);
>
> I want to rule this one out. I don't like to think to much about a
> single line of code. Call me lazy. :)
>
>> which I find better than having two "read" lines:
>> >> +             bytes_left = file.size - file.suffixlen - bytes_sent;
>> >> +             if (bytes_left <  xfer_size)
>> >> +                     ret = read(file.fd, buf, bytes_left);
>> >> +             else
>> >> +                     ret = read(file.fd, buf, xfer_size);
>
> Not to bad but I like the first one better.
>
>> Voting is open :)
>
> :)
>
> regards
> Stefan Schmidt
>



More information about the devel mailing list