| Message ID | 20200330070159.50077-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs-progs: check: Sanitize the return value for qgroup error | expand |
On 30.03.20 г. 10:01 ч., Qu Wenruo wrote: > [BUG] > btrfs check can return strange return value for shell: > [Inferior 1 (process 48641) exited with code 0213] > ^^^^ > > [CAUSE] > It's caused by the incorrect handling of qgroup error. > > qgroup_report_ret can be -117 (-EUCLEAN), using that value with exit() > can cause overflow, causing return value not properly recognized. > > [FIX] > Fix it by sanitize the return value to 0 or 1. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > check/main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/check/main.c b/check/main.c > index 37c5b35a36bd..8e29337c5c23 100644 > --- a/check/main.c > +++ b/check/main.c > @@ -10458,7 +10458,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv) > goto out; > } > if (qgroup_report_ret && (!qgroups_repaired || ret)) > - err |= qgroup_report_ret; > + err |= !!qgroup_report_ret; The original code was just "wow". qgroup_report_ret was an ordinary engative error code and it was "ORed" with err ... Just wow... While this is correct absed on what the function is doing it's awful. WHy can't simply err = ret in every error branch? Err is really used as a boolean and frankly instead of doing err = blah or err = true we have err |= !!blah. Seriously? Was this code being primed for submission to https://www.ioccc.org/ ? Sheesh... in any case: Reviewed-by: Nikolay Borisov <nborisov@suse.com> > ret = 0; > } else { > fprintf(stderr, >
On Mon, Mar 30, 2020 at 03:01:59PM +0800, Qu Wenruo wrote: > [BUG] > btrfs check can return strange return value for shell: > [Inferior 1 (process 48641) exited with code 0213] > ^^^^ > > [CAUSE] > It's caused by the incorrect handling of qgroup error. > > qgroup_report_ret can be -117 (-EUCLEAN), using that value with exit() > can cause overflow, causing return value not properly recognized. > > [FIX] > Fix it by sanitize the return value to 0 or 1. > > Signed-off-by: Qu Wenruo <wqu@suse.com> Added to devel, thanks.
diff --git a/check/main.c b/check/main.c index 37c5b35a36bd..8e29337c5c23 100644 --- a/check/main.c +++ b/check/main.c @@ -10458,7 +10458,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv) goto out; } if (qgroup_report_ret && (!qgroups_repaired || ret)) - err |= qgroup_report_ret; + err |= !!qgroup_report_ret; ret = 0; } else { fprintf(stderr,
[BUG] btrfs check can return strange return value for shell: [Inferior 1 (process 48641) exited with code 0213] ^^^^ [CAUSE] It's caused by the incorrect handling of qgroup error. qgroup_report_ret can be -117 (-EUCLEAN), using that value with exit() can cause overflow, causing return value not properly recognized. [FIX] Fix it by sanitize the return value to 0 or 1. Signed-off-by: Qu Wenruo <wqu@suse.com> --- check/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)