Skip to content

parse: ignore leading minus sign when splitting options #2112

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cenhuil wants to merge 1 commit into
base: master
Choose a base branch
from
+8 −1
Open

parse: ignore leading minus sign when splitting options #2112

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1041,8 +1041,15 @@ static int handle_option(const struct fio_option *o, const char *__ptr,
if (o->type != FIO_OPT_STR_MULTI && o->type != FIO_OPT_RANGE) {
if (!ptr2)
ptr2 = strchr(ptr, ':');
if (!ptr2)
if (!ptr2) {
ptr2 = strchr(ptr, '-');
/*
* If the first character is a minus sign, it's
* likely a negative number, not a delimiter.
*/

@sitsofe sitsofe Jun 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will - ever be a delimiter when we aren't parsing a FIO_OPT_RANGE? I think we can only be parsing {FIO_OPT_STR_VAL, FIO_OPT_STR_VAL_TIME, FIO_OPT_INT, FIO_OPT_ULL, FIO_OPT_BOOL, FIO_OPT_STR_SET, FIO_OPT_STR_VAL_ZONE} at this point...

@cenhuil cenhuil Jun 25, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, - is indeed used as a delimiter for those other types as well.

For example, options like bs, ba, or rate (which are evaluated as FIO_OPT_ULL or FIO_OPT_INT) allow users to specify different values for read, write, and trim. According to the fio documentation and the current parsing logic, these multiple values can be delimited by commas, colons, or hyphens.

Therefore, a command like fio --name=test --bs=4k-8k is perfectly valid and successfully assigns 4k to Read and 8k to Write by using the hyphen as a delimiter.

If we completely remove ptr2 = strchr(ptr, '-'); for types other than FIO_OPT_RANGE, it would break multi-value assignments like bs=4k-8k. That is why we still need to support - as a delimiter here, but merely skip it if it's the very first character (acting as a negative sign).

@sitsofe sitsofe Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well this has turned out to be trickier than I first thought! From the documentation for "int":

If the option accepts an upper and lower range, use a colon ‘:’ or minus ‘-’ to separate such values. See irange.

If someone does bs=4k-8k I think they are confused because it looks like a range (so they should have used bsrange) but as you stated it will cause reads to be 4k and writes to be 8k. The docs for blocksize define it as taking a comma separated values and not a range. It's similar for blockalign and rate.

@vincentkfu I'm torn because the docs do describe this strange behaviour ("minus as a separator/delimiter") and taking it out will break backwards compatibility if someone has chosen to use it that way but isn't keeping it confusing? If we do say we keep this behaviour I see this PR as the only way to support negative numbers on things that aren't strings or floats.

@vincentkfu vincentkfu Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see in the documentation where - should be used as a delimiter outside of places where a range is expected. I think treating - as a delimiter is just a quirk of the parsing logic. Ideally Fio would print an error message when it encounters a - when a comma is expected.
I'm inclined to allow the current undefined (as far as I could find) and confusing behavior to remain and think will take a closer look at the fix as time allows.

@sitsofe sitsofe Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see in the documentation where - should be used as a delimiter outside of places where a range is expected.

Technically the docs describe this bizarre behaviour for things that take int parameter type. Perhaps we should change that text to leave the range stuff entirely to the irange description...

I'm inclined to allow the current undefined (as far as I could find) and confusing behavior to remain and think will take a closer look at the fix as time allows.

That seems fair.

if (ptr2 == ptr)
ptr2 = strchr(ptr + 1, '-');
}
}
} else if (ptr && o->type == FIO_OPT_FLOAT_LIST) {
ptr2 = strchr(ptr, ':');
Expand Down