gh-Cyan4973/xxHash

Watch

Fork 0

Parse args like shasum #428

New issue

Closed

opened 2020-07-14 00:43:45 -05:00 by jaki · 1 comment

jaki commented

2020-07-14 00:43:45 -05:00

(Migrated from github.com)

Since xxhash is cross-platform compatible, I assume it tries to stay away from using getopt, which shasum uses (see coreutils src/md5sum.c main func). The current argument parsing of xxhsum has some bugs and some enhancements it could use:

BUG: Don't double-interpret arguments as option and filename
ENHANCEMENT: Interpret -- as end of option parsing

For 1., try

touch file file2
./xxhsum file >file.xxhsum
./xxhsum file.xxhsum --check

and get

file: OK
Error: Could not open '--check': No such file or directory

Now, try

./xxhsum file2 >--check
./xxhsum file.xxhsum --check

and get

file: OK
file2: OK

I'm pretty sure we don't want to be double-interpreting --check as an option and filename.

For 2., try

touch -- -_-
./xxhsum -_-

and get

Wrong parameters
...

Try

./xxhsum -- -_-

and get

Wrong parameters
...

So there appears to be no way to run xxshum on a file that begins with -. Sure, we can pass it through stdin (e.g. cat -_- | xxhsum), but that's not ideal (and you also need to attach the filename to the output).

I tried working on these myself, but it's nontrivial because the current code mostly uses an int variable filenamesStart to determine where to start processing filenames. I don't really have experience with getopt, but I assume it would be the best solution (but then, we need to find an alternative for Windows systems, use #ifdef _WIN32, etc.).

Here are other examples of what shasum does along with their not-working xxhsum equivalents in comments:

touch a b
shasum a -- b # xxhsum a -- b

da39a3ee5e6b4b0d3255bfef95601890afd80709  a
da39a3ee5e6b4b0d3255bfef95601890afd80709  b

touch -- --
shasum -- -- # xxhsum -- --

da39a3ee5e6b4b0d3255bfef95601890afd80709  --

touch a b
shasum a --algorithm 224 b # xxhsum a -H32 b

d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f  a
d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f  b

Since xxhash is cross-platform compatible, I assume it tries to stay away from using `getopt`, which `shasum` uses (see coreutils `src/md5sum.c` main func). The current argument parsing of xxhsum has some bugs and some enhancements it could use: 1. BUG: Don't double-interpret arguments as option and filename 2. ENHANCEMENT: Interpret `--` as end of option parsing For 1., try ```sh touch file file2 ./xxhsum file >file.xxhsum ./xxhsum file.xxhsum --check ``` and get ``` file: OK Error: Could not open '--check': No such file or directory ``` Now, try ```sh ./xxhsum file2 >--check ./xxhsum file.xxhsum --check ``` and get ``` file: OK file2: OK ``` I'm pretty sure we don't want to be double-interpreting `--check` as an option and filename. For 2., try ```sh touch -- -_- ./xxhsum -_- ``` and get ``` Wrong parameters ... ``` Try ```sh ./xxhsum -- -_- ``` and get ``` Wrong parameters ... ``` So there appears to be no way to run `xxshum` on a file that begins with `-`. Sure, we can pass it through stdin (e.g. `cat -_- | xxhsum`), but that's not ideal (and you also need to attach the filename to the output). I tried working on these myself, but it's nontrivial because the current code mostly uses an `int` variable `filenamesStart` to determine where to start processing filenames. I don't really have experience with `getopt`, but I assume it would be the best solution (but then, we need to find an alternative for Windows systems, use `#ifdef _WIN32`, etc.). Here are other examples of what `shasum` does along with their not-working `xxhsum` equivalents in comments: ```sh touch a b shasum a -- b # xxhsum a -- b ``` ``` da39a3ee5e6b4b0d3255bfef95601890afd80709 a da39a3ee5e6b4b0d3255bfef95601890afd80709 b ``` ```sh touch -- -- shasum -- -- # xxhsum -- -- ``` ``` da39a3ee5e6b4b0d3255bfef95601890afd80709 -- ``` ```sh touch a b shasum a --algorithm 224 b # xxhsum a -H32 b ``` ``` d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f a d14a028c2a3a2bc9476102bb288234c415a2b01f828ea62ac5b3e42f b ```

👍 1

Cyan4973 commented

2020-07-14 00:53:05 -05:00

(Migrated from github.com)

The current xxhsum has a pretty strong limitation which requires that all commands must appear first on the command line, then all following arguments are interpreted as filenames (that's the filenamesStart variable). Changing that restriction to allow files and commands to be provided in any order would require to add some substantial complexity, which I would prefer to avoid.

Beyond that, the other suggestions are fair points, and can be provided.
Supporting filenames starting with -, or interpreting -- as "from now on all arguments are filenames" are within reach.

The current `xxhsum` has a pretty strong limitation which requires that all commands must appear first on the command line, then all following arguments are interpreted as filenames (that's the `filenamesStart` variable). Changing that restriction to allow files and commands to be provided in any order would require to add some substantial complexity, which I would prefer to avoid. Beyond that, the other suggestions are fair points, and can be provided. Supporting filenames starting with `-`, or interpreting `--` as "from now on all arguments are filenames" are within reach.

No milestone

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

gh-Cyan4973/xxHash#428

No description provided.

Rows
Columns