[RFC] Drop md5sum, crc32 and sha1 cmds in favor of hash cmd
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!
Regards, Igor
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.
Hi Tom,
On Wed, Feb 7, 2024 at 2:48 PM Tom Rini trini@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++; } ... }
-- Tom
Regards, Igor
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@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.
On Wed, 7 Feb 2024 at 13:48, Tom Rini trini@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.
Yes, I actually have that one my todo list to basically alias the individual commands to the hash command to remove those commands. My intention there was a first step to allow us to eventually minimise or even remove the use of obsolete hashes etc.
Hello Tom, Peter,
On Wed, Feb 7, 2024 at 3:16 PM Peter Robinson pbrobinson@gmail.com wrote:
On Wed, 7 Feb 2024 at 13:48, Tom Rini trini@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.
Yes, I actually have that one my todo list to basically alias the individual commands to the hash command to remove those commands. My intention there was a first step to allow us to eventually minimise or even remove the use of obsolete hashes etc.
Thank you for your comments. I'll re-factor as discussed and send a patch series when it's ready (plan to do that before the next merge window opens).
Regards, Igor
participants (3)
-
Igor Opaniuk -
Peter Robinson -
Tom Rini