kernel/sysctl.c: about the return value after fail call
cover kernel/sysctl.c: about the return value after fail call
Patch 1 for bug fix: check return value after call proc_put_char() in __do_proc_doulongvec_minmax()
Patch 2 for simplify code : Simplify code for C code readers.
--- kernel/sysctl.c | 30 +++++++++++++++++------------- 1 files changed, 17 insertions(+), 13 deletions(-)
PATCH 1/2 kernel/sysctl.c: check return value after call proc_put_char() in __do_proc_doulongvec_minmax()
Need check the return value of proc_put_char(), just like another have done in __do_proc_doulongvec_minmax().
--- kernel/sysctl.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
kernel/sysctl.c +4 -1
@@ -2214,8 +2214,11 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int *i = val; }else { val = convdiv * (*i) / convmul; -if (!first) +if (!first) { err =proc_put_char (&buffer, &left,'\t' ); +if (err) + goto free; + } err =proc_put_long (&buffer, &left, val, false);if (err)break ;
Signature · 1 line
1.7.7.6
PATCH 2/2 kernel/sysctl.c: simplify code for C code readers
In related functions, they have already had tag 'free', so use 'goto' instead of 'break' to keep one (not multiple) style in each function.
Also remove useless checking 'err' code or move it to related 'if' code block.
--- kernel/sysctl.c | 25 +++++++++++++------------ 1 files changed, 13 insertions(+), 12 deletions(-)
kernel/sysctl.c +13 -12
@@ -1958,27 +1958,28 @@ static int __do_proc_dointvec(void *tbl_data, struct ctl_table *table, proc_wspace_sep, sizeof(proc_wspace_sep), NULL);if (err) -break ; + goto free;if (conv (&neg, &lval, i,1 , data)) { err = -EINVAL ; -break ; + goto free; } }else {if (conv (&neg, &lval, i,0 , data)) { err = -EINVAL ; -break ; + goto free; } -if (!first) +if (!first) { err =proc_put_char (&buffer, &left,'\t' ); -if (err) -break ; +if (err) + goto free; + } err =proc_put_long (&buffer, &left, lval, neg);if (err) -break ; + goto free; } } -if (!write && !first && left && !err) +if (!write && !first && left) err =proc_put_char (&buffer, &left,'\n' );if (write && !err && left) left -=proc_skip_spaces (&kbuf);
@@ -2206,7 +2207,7 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int proc_wspace_sep, sizeof(proc_wspace_sep), NULL);if (err) -break ; + goto free;if (neg)continue ;if ((min && val < *min) || (max && val > *max))
@@ -2221,11 +2222,11 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int } err =proc_put_long (&buffer, &left, val, false);if (err) -break ; + goto free; } } -if (!write && !first && left && !err) +if (!write && !first && left) err =proc_put_char (&buffer, &left,'\n' );if (write && !err) left -=proc_skip_spaces (&kbuf);
@@ -2515,7 +2516,7 @@ int proc_do_large_bitmap(struct ctl_table *table, int write,return -ENOMEM ; }proc_skip_char (&kbuf, &left,'\n' ); -while (!err && left) { +while (left) {unsigned long val_a, val_b;bool neg;
Signature · 1 line
1.7.7.6
Re: [PATCH 1/2] kernel/sysctl.c: check return value after call proc_put_char() in __do_proc_doulongvec_minmax()
In reply to Chen GangQuoted 13 lines · Chen Gang
Need check the return value of
proc_put_char(), just like another have done in__do_proc_doulongvec_minmax().Signed-off-by Chen Gang <[email protected]>--- kernel/sysctl.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)kernel/sysctl.c +0
-0
kernel/sysctl.c static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int · line 2221
Quoted 9 lines
kernel/sysctl.c +4
-1@@ -2214,8 +2214,11 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int *i = val; }else { val = convdiv * (*i) / convmul; -if (!first) +if (!first) { err =proc_put_char (&buffer, &left,'\t' ); +if (err) + goto free; + }
I think you should use break and convert it in
to goto free in patch 2/2.
Otherwise, this style looks out of place.
Quoted 3 lines
kernel/sysctl.c +0
-0err =proc_put_long (&buffer, &left, val, false);if (err)break ;
Re: [PATCH 1/2] kernel/sysctl.c: check return value after call proc_put_char() in __do_proc_doulongvec_minmax()
Quoted 27 lines · Joe Perches
Quoted 13 lines · Chen Gang
Need check the return value of
proc_put_char(), just like another have done in__do_proc_doulongvec_minmax().Signed-off-by Chen Gang <[email protected]>--- kernel/sysctl.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)kernel/sysctl.c +0
-0kernel/sysctl.c static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int · line 2221
Quoted 9 lines
kernel/sysctl.c +4
-1@@ -2214,8 +2214,11 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int *i = val; }else { val = convdiv * (*i) / convmul; -if (!first) +if (!first) { err =proc_put_char (&buffer, &left,'\t' ); +if (err) + goto free; + }I think you should use break and convert it in
to goto free in patch 2/2.
Quoted 2 lines
Otherwise, this style looks out of place.
Hmm... It sounds reasonable to me, if necessary, I will send patch v2 for them (no reply means not need send patch v2, this time).
Thanks.
Quoted 8 lines
Quoted 3 lines
kernel/sysctl.c +0
-0err =proc_put_long (&buffer, &left, val, false);if (err)break ;
Signature · 1 line
Chen Gang
PATCH 1/1 kernel/sysctl.c: check return value after call proc_put_char() in __do_proc_doulongvec_minmax()
Need check the return value of proc_put_char(), just like another have done in __do_proc_doulongvec_minmax().
--- kernel/sysctl.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
kernel/sysctl.c +4 -1
@@ -2214,8 +2214,11 @@ static int __do_proc_doulongvec_minmax(void *data, struct ctl_table *table, int *i = val; }else { val = convdiv * (*i) / convmul; -if (!first) +if (!first) { err =proc_put_char (&buffer, &left,'\t' ); +if (err) +break ; + } err =proc_put_long (&buffer, &left, val, false);if (err)break ;
Signature · 1 line
1.7.7.6