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 <tr...@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
signature.asc
Description: PGP signature