It is a normal case that bitmask parameter is 0, so pre-initialize the
names[] to null string to cover this case.
Also remove the superfluous "+1" in names[sizeof(sys_info_avail) + 1],
which is needed for 'strlen()', but not for 'sizeof()'.
Suggested-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
lib/sys_info.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/lib/sys_info.c b/lib/sys_info.c
index 5bf503fd7ec1..b2df148971ba 100644
--- a/lib/sys_info.c
+++ b/lib/sys_info.c
@@ -55,7 +55,7 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
void *buffer, size_t *lenp,
loff_t *ppos)
{
- char names[sizeof(sys_info_avail) + 1];
+ char names[sizeof(sys_info_avail)];
struct ctl_table table;
unsigned long *si_bits_global;
@@ -81,6 +81,9 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
char *delim = "";
int i, len = 0;
+ /* *si_bits_glabl could be 0 */
+ names[0] = '\0';
+
for (i = 0; i < ARRAY_SIZE(si_names); i++) {
if (*si_bits_global & si_names[i].bit) {
len += scnprintf(names + len, sizeof(names) - len,
--
2.43.5
On Fri 2025-08-15 15:14:26, Feng Tang wrote:
> It is a normal case that bitmask parameter is 0, so pre-initialize the
> names[] to null string to cover this case.
>
> Also remove the superfluous "+1" in names[sizeof(sys_info_avail) + 1],
> which is needed for 'strlen()', but not for 'sizeof()'.
>
> --- a/lib/sys_info.c
> +++ b/lib/sys_info.c
> @@ -55,7 +55,7 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> void *buffer, size_t *lenp,
> loff_t *ppos)
> {
> - char names[sizeof(sys_info_avail) + 1];
> + char names[sizeof(sys_info_avail)];
> struct ctl_table table;
> unsigned long *si_bits_global;
>
> @@ -81,6 +81,9 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> char *delim = "";
> int i, len = 0;
>
> + /* *si_bits_glabl could be 0 */
s/si_bits_glabl/si_bits_global/
But I would personally remove the comment completely. IMHO, the
purpose is quite obvious. But I do not resist on it.
> + names[0] = '\0';
> +
> for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> if (*si_bits_global & si_names[i].bit) {
> len += scnprintf(names + len, sizeof(names) - len,
Otherwise, it looks good to me:
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
On Tue, Aug 19, 2025 at 10:34:14AM +0200, Petr Mladek wrote:
> On Fri 2025-08-15 15:14:26, Feng Tang wrote:
> > It is a normal case that bitmask parameter is 0, so pre-initialize the
> > names[] to null string to cover this case.
> >
> > Also remove the superfluous "+1" in names[sizeof(sys_info_avail) + 1],
> > which is needed for 'strlen()', but not for 'sizeof()'.
> >
> > --- a/lib/sys_info.c
> > +++ b/lib/sys_info.c
> > @@ -55,7 +55,7 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> > void *buffer, size_t *lenp,
> > loff_t *ppos)
> > {
> > - char names[sizeof(sys_info_avail) + 1];
> > + char names[sizeof(sys_info_avail)];
> > struct ctl_table table;
> > unsigned long *si_bits_global;
> >
> > @@ -81,6 +81,9 @@ int sysctl_sys_info_handler(const struct ctl_table *ro_table, int write,
> > char *delim = "";
> > int i, len = 0;
> >
> > + /* *si_bits_glabl could be 0 */
>
> s/si_bits_glabl/si_bits_global/
>
> But I would personally remove the comment completely. IMHO, the
> purpose is quite obvious. But I do not resist on it.
Will remove.
>
> > + names[0] = '\0';
> > +
> > for (i = 0; i < ARRAY_SIZE(si_names); i++) {
> > if (*si_bits_global & si_names[i].bit) {
> > len += scnprintf(names + len, sizeof(names) - len,
>
> Otherwise, it looks good to me:
>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
Thank you! will resend a v2 patchset.
- Feng
>
> Best Regards,
> Petr
© 2016 - 2026 Red Hat, Inc.