Messages in this thread | | | Subject | Re: [PATCH v2] EDAC/mc_sysfs: Add missing newlines when printing {max,dimm}_location | From | Xiongfeng Wang <> | Date | Fri, 18 Sep 2020 10:37:28 +0800 |
| |
On 2020/9/18 0:25, Borislav Petkov wrote: > On Thu, Sep 17, 2020 at 07:38:57PM +0800, Xiongfeng Wang wrote: >> I am not sure if snprintf will return a value larger than its second input >> paramter 'size'. > > The comment over snprintf() says > > * The return value is the number of characters which would be > * generated for the given input, excluding the trailing null, > * as per ISO C99. If the return is greater than or equal to > * @size, the resulting string is truncated. > > And let's try it, see diff at the end. Now look what that produces: > > [ 2.594796] kernel_init: len: 16, str: [A lo] > > it returns 16 for len even though the buffer is 5 chars long. So in our > patch, we'd increment by 16 which would be wrong. > > Now let's use scnprintf(): > > [ 2.700142] kernel_init: len: 4, str: [A lo] > > Much better. Lemme do that. > >> Not sure whether we need to check 'len' equals to 0. >> if (len <= 0) ? > > Yeah, lemme fix that too, so we have now incrementally ontop:
Thansk a lot. I will send another version. Also I will change the 'snprintf' in 'dimmdev_location_show()' to 'scnprintf'
Thanks, Xiongfeng
> > --- > diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c > index fa0551c81e63..c56e0004b39e 100644 > --- a/drivers/edac/edac_mc_sysfs.c > +++ b/drivers/edac/edac_mc_sysfs.c > @@ -822,18 +822,17 @@ static ssize_t mci_max_location_show(struct device *dev, > int i, n; > > for (i = 0; i < mci->n_layers; i++) { > - n = snprintf(p, len, "%s %d ", > - edac_layer_name[mci->layers[i].type], > - mci->layers[i].size - 1); > - > + n = scnprintf(p, len, "%s %d ", > + edac_layer_name[mci->layers[i].type], > + mci->layers[i].size - 1); > len -= n; > - if (len < 0) > + if (len <= 0) > goto out; > > p += n; > } > > - p += snprintf(p, len, "\n"); > + p += scnprintf(p, len, "\n"); > out: > return p - data; > } > --- > > Test diff: > > --- > diff --git a/init/main.c b/init/main.c > index ae78fb68d231..e2d6110d3a3d 100644 > --- a/init/main.c > +++ b/init/main.c > @@ -1397,7 +1397,8 @@ void __weak free_initmem(void) > > static int __ref kernel_init(void *unused) > { > - int ret; > + char str[5]; > + int ret, len; > > kernel_init_freeable(); > /* need to finish all async __init code before freeing the memory */ > @@ -1419,6 +1420,11 @@ static int __ref kernel_init(void *unused) > > do_sysctl_args(); > > + len = snprintf(str, 5, "A longer string\n"); > + > + pr_info("%s: len: %d, str: [%s]\n", > + __func__, len, str); > + > if (ramdisk_execute_command) { > ret = run_init_process(ramdisk_execute_command); > if (!ret) >
| |