[U-Boot] [PATCH 16/20] Roll crc32 into hash infrastructure

[Simon Glass]

Hi Wolfgang,

On Mon, Feb 18, 2013 at 3:35 AM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Tom,
>
> In message <51216721.1010603 at ti.com> you wrote:
>>
>> There's another thread I don't have yet (and I don't have this one in
>> gmail yet even).  But, I am OK with custodians using their repos, but
>> not the master branch, for unrelated but otherwise good patches. I'm
>> also fine with patchwork bundles.  I suppose we could use the staging
>> repository for these changes instead.
>
> What I mostly object about there is that these patches would go into
> mainline basicly unreviewed, as patch submission and pull request is
> all done from a single person, with no other feedback on the patches
> at all.  And this affects a lot of common code...

Fair enough. I suspect a number of people scan the code, but few feel
invested enough to formally Ack it. Also, providing a full review of
such a series can take quite a bit of time. Against that, I think it
is better to get code in and tested than have it sit around until just
before the next release.

>
> Actually, I see this change when pulling u-boot-x86.git/master:

Thanks for looking at it. Some of it is just the code moving around,
but I will take a look at why hash_command grows so much, and why the
overall size has grown. Clearly I didn't do enough here when I checked
the series. One change was trying to unify the verify feature, so
perhaps something has gone wrong there.

>
> -> bloat-o-meter u-boot-before u-boot
> add/remove: 9/0 grow/shrink: 3/14 up/down: 1006/-560 (446)
> function                                     old     new   delta
> hash_command                                   -     424    +424
> strncasecmp                                    -     156    +156
> simple_itoa                                    -     104    +104
> crc32_wd_buf                                   -      76     +76
> setenv_hex                                     -      68     +68
> setenv_ulong                                   -      52     +52
> strcasecmp                                     -      36     +36
> do_mem_loopw                                 304     328     +24
> static.local                                   -      22     +22
> do_mem_loop                                  268     288     +20
> hash_algo                                      -      16     +16
> do_mem_cmp                                   332     340      +8
> do_mem_mw                                    224     220      -4
> set_working_fdt_addr                          72      52     -20
> load_serial_ymodem                           300     280     -20
> load_serial                                  512     492     -20
> index_partitions                             200     180     -20
> do_load_serial_bin                          1844    1824     -20
> do_load                                      468     448     -20
> do_jffs2_fsload                              320     300     -20
> do_imgextract                                636     592     -44
> NetLoop                                      832     788     -44
> do_mem_cp                                    312     252     -60
> do_bootm                                    1244    1180     -64
> do_mem_crc                                   188      88    -100
> do_mem_mtest                                1436    1332    -104
>
>
> So there are changes all over the place, including a growth of the
> memory footprint.  I think this needs at least minimal review.

We need more reviewers I think.

Regards,
Simon

>
> 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
> Time is an illusion perpetrated by the manufacturers of space.