search

AgentD / squashfs-tools-ng

A new set of tools and libraries for working with SquashFS images
Other
194 stars 30 forks source link

Incosistent option recognition #48

Closed aparcar closed 4 years ago

aparcar commented 4 years ago

Hi, I'm trying to use gensquashfs to create or overwrite a file, however the usage of -f aka --force is inconsistent or broken. Below an example in which the folder root-ath79 exists:

# -f at the end - fail
gensquashfs -b 1M --all-root --pack-dir root-ath79/ root.squashfs -f
No input file or directory specified.
Try `gensquashfs --help' for more information.

# -f before --pack-dir - fail
gensquashfs -b 1M --all-root -f --pack-dir root-ath79 root.squashfs
root.squashfs: File exists

# -f before and after --pack-dir - works
gensquashfs -b 1M --all-root -f --pack-dir root-ath79/ root.squashfs -f
packing bin/board_detect
packing bin/busybox
packing bin/config_generate
packing bin/ipcalc.sh
[...]
AgentD commented 4 years ago

Thanks! The bug you uncovered in the command line processing is actually in --all-root and not in --force. It appears I accidentally set the required_argument flag (copy & paste error) and --pack-dir immediately afterwards was recognized as argument and skipped.

I fixed this in commit d9b79c78f4ceac5a6a271b61f7addb02d504efd8

Commit d43b31df6b84e54d0d657fd27b2f2b5734d8f162 adds a test case for this exact scenario.

Regarding the order of options and arguments: Unlinke mksquashfs, the tools in this package all use getopt_long to do option parsing.

The filename is assumed to be at argv[optind] after option parsing is done, so it always needs to be at the end. See bin/gensquashfs/options.c

In commit 5380c605440db063f8b80a2a48b326a8ce9b0d93, I patched gensquashfs to reject unexpected extra arguments after option processing is done to clearly indicate that this is an error (same for tar2sqfs).

# -f at the end - fail
gensquashfs -b 1M --all-root --pack-dir root-ath79/ root.squashfs -f
No input file or directory specified.
Try `gensquashfs --help' for more information.

The bug is in --all-root causing the --pack-dir to be consumed as an argument and skipped. Even in the fixed version, the -f option will be ignored, because it's after the output file name and no longer recognized as option.

# -f before --pack-dir - fail
gensquashfs -b 1M --all-root -f --pack-dir root-ath79 root.squashfs
root.squashfs: File exists

Same bug, but this time, the -f is consumed as an argument to --all-root and the pack dir is recognized.

# -f before and after --pack-dir - works
gensquashfs -b 1M --all-root -f --pack-dir root-ath79/ root.squashfs -f
packing bin/board_detect
packing bin/busybox
packing bin/config_generate
packing bin/ipcalc.sh
[...]

Are you sure the output file exists? The -f option after the file name cannot possibly be recognized since getopt_long stops option processing right at root.squashfs.