Messages in this thread Patches in this message | | | Date | Thu, 17 Sep 2020 18:25:37 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v2] EDAC/mc_sysfs: Add missing newlines when printing {max,dimm}_location |
| |
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:
--- 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) -- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |