[RFC] Drop md5sum, crc32 and sha1 cmds in favor of hash cmd

[Tom Rini]

On Wed, Feb 07, 2024 at 03:10:01PM +0100, Igor Opaniuk wrote:
> Hi Tom,
> 
> On Wed, Feb 7, 2024 at 2:48 PM Tom Rini <trini at konsulko.com> wrote:
> >
> > On Wed, Feb 07, 2024 at 02:00:16PM +0100, Igor Opaniuk wrote:
> > > Hello,
> > >
> > > I was playing a bit with different hash functions recently, and
> > > it turned out that md5sum, crc32, sha1 cmds just duplicate
> > > what is already covered by generic `hash` cmd.
> > >
> > > => sha1 0x60000000 0x200
> > > sha1 for 60000000 ... 600001ff ==> 4ff5ffc91d00a95155518b920f46e2483d0e1437
> > > => hash sha1 0x60000000 0x200
> > > sha1 for 60000000 ... 600001ff ==> 4ff5ffc91d00a95155518b920f46e2483d0e1437
> > >
> > > => crc32 0x60000000 0x200
> > > crc32 for 60000000 ... 600001ff ==> 6fe352e8
> > > => hash crc32 0x60000000 0x200
> > > crc32 for 60000000 ... 600001ff ==> 6fe352e8
> > >
> > > => md5sum 0x60000000 0x200
> > > md5 for 60000000 ... 600001ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
> > > => hash md5 0x60000000 0x200
> > > md5 for 60000000 ... 600001ff ==> e6bbbe95f5b41996f4a9b9af7bbd4050
> > >
> > > Considering that most of them (besides md5sum) are using the same
> > > int hash_command() function under the hood, but have a lot of duplicated
> > > code for handling params, does it make sense to do some cleanup and
> > > drop all them in favour  `hash`?
> > >
> > > I also plan to extend usage info for `hash` by adding a list
> > > compiled-in algos based on hash related compiled flags
> > > (CONFIG_SHA1, CONFIG_CRC32 etc), so it's clear what algos
> > > are available for hash calculation.
> > >
> > > Comments/objections are welcome!
> >
> > It would be good, implementation wise, if each of those commands was
> > just a redirect to hash ..., similar to how "load ...." will call the
> > right filesystem calls. Does that make sense? Thanks.
> 
> Sure. You mean something like this?:
> 
> /* cmd/crc32.c */
> int do_crc32(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> {
>         return do_hash(cmdtp, flag, argc, argv);
> }
> 
> /* cmd/hash.c */
> static int do_hash(struct cmd_tbl *cmdtp, int flag, int argc,
>                              char *const argv[]) {
> ...
>         /* if not true, we are called from crc32/sha1/etc alias */
>         if (!strcmp(argv[0], "hash")) {
>                 /*
>                  * Hash cmd is used directly,
>                  * Move forward to 'algorithm' parameter
>                  */
>                 argc--;
>                 argv++;
>         }
> ...
> }

So right, sorry, I didn't quite follow what you said before. If there's
further cleanup we can do with how hash_command is abstracting things,
yes, lets.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240207/b5ff3454/attachment.sig>