From: Nithurshen <nithurshen.dev@gmail.com>
To: singhutkal015@gmail.com
Cc: hsiangkao@linux.alibaba.com, linux-erofs@lists.ozlabs.org,
xiang@kernel.org
Subject: Re: [PATCH] fsck: add --workers option to configure worker threads
Date: Sun, 22 Mar 2026 14:06:20 +0530 [thread overview]
Message-ID: <20260322083620.19933-1-nithurshen.dev@gmail.com> (raw)
In-Reply-To: <20260322064706.27001-1-singhutkal015@gmail.com>
Hi Utkal,
I've run some tests on your patch.
I compiled fsck.erofs locally, generated a test EROFS image, and passed
various valid and invalid inputs to the new --workers flag (positive
integers, hex, trailing garbage, overflow, and negative numbers).
The happy path works perfectly. The *endptr check correctly catches
trailing garbage, and standard overflows are handled well.
I would suggest changes:
There's a hidden edge case with negative numbers (like --workers=-1)
on 32-bit systems. Because strtoul() is used, a negative number wraps
around to ULONG_MAX. On 64-bit machines, this is safely caught by your
v > UINT_MAX check. However, on 32-bit machines, ULONG_MAX equals
UINT_MAX, meaning -1 will bypass the check and attempt to spawn
4.2 billion threads.
Also, when -EINVAL is returned, fsck silently prints the generic help
menu without explaining what went wrong.
Therefore, in case you decide to send a v2 patch,
1. Switch to strtol() instead of strtoul() so you can explicitly catch
negative numbers or zero (e.g., `if (*endptr || v <= 0 || ...)`).
2. Add an explicit error message (e.g., using erofs_err) before
returning -EINVAL so the user knows their input was invalid.
These are just my opinion, but follow what the lead maintainers say.
Best,
Nithurshen
next prev parent reply other threads:[~2026-03-22 8:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-22 6:47 [PATCH] fsck: add --workers option to configure worker threads Utkal Singh
2026-03-22 8:36 ` Nithurshen [this message]
2026-03-22 8:57 ` Nithurshen
2026-03-22 9:31 ` Utkal Singh
2026-03-22 9:48 ` Gao Xiang
2026-03-22 16:23 ` Utkal Singh
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260322083620.19933-1-nithurshen.dev@gmail.com \
--to=nithurshen.dev@gmail.com \
--cc=hsiangkao@linux.alibaba.com \
--cc=linux-erofs@lists.ozlabs.org \
--cc=singhutkal015@gmail.com \
--cc=xiang@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.