Messages in this thread |  | | Date | Sat, 7 Jan 2023 15:08:51 +0100 | Subject | Re: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc | From | Christophe JAILLET <> |
| |
Le 07/01/2023 à 13:23, Quanfa Fu a écrit : > On Sat, Jan 7, 2023 at 4:42 PM Christophe JAILLET > <christophe.jaillet@wanadoo.fr> wrote: >> >> Le 07/01/2023 à 04:45, Quanfa Fu a écrit : >>> Since this memory will be filled soon below, I feel that there is >>> no need for a memory of all zeros here. 'snprintf' does not return >>> negative num according to ISO C99, so I feel that no judgment is >>> needed here. >>> >>> No functional change intended. >>> >>> Signed-off-by: Quanfa Fu <quanfafu@gmail.com> >>> --- >>> kernel/trace/trace_eprobe.c | 4 +--- >>> 1 file changed, 1 insertion(+), 3 deletions(-) >>> >>> diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c >>> index 352b65e2b910..cd1d271a74e7 100644 >>> --- a/kernel/trace/trace_eprobe.c >>> +++ b/kernel/trace/trace_eprobe.c >>> @@ -917,15 +917,13 @@ static int trace_eprobe_parse_filter(struct trace_eprobe *ep, int argc, const ch >>> for (i = 0; i < argc; i++) >>> len += strlen(argv[i]) + 1; >>> >>> - ep->filter_str = kzalloc(len, GFP_KERNEL); >>> + ep->filter_str = kmalloc(len, GFP_KERNEL); >>> if (!ep->filter_str) >>> return -ENOMEM; >>> >>> p = ep->filter_str; >>> for (i = 0; i < argc; i++) { >>> ret = snprintf(p, len, "%s ", argv[i]); >>> - if (ret < 0) >>> - goto error; >>> if (ret > len) { >> > > I think that: > > for example, argc = 2, argv = {"a", "b"}; > > Before the loop > =============== > len = (1 + 1) + (1 + 1) = 4; > ep->filter_str = 0x00 0x00 0x00 0x00 > ^ > |__ p > > After the loop: > =============== > i = 1, snprintf write: 'a' and ' ', so ret = 2 > i = 2, snprintf write: 'b' and ' ', so ret = 2 > ===============
Ok, I missed the space after the %s.
Sorry for the noise.
> Since the length of the last argv is not enough > to write, the space is replaced by null > > ep->filter_str = 0x61 0x20 0x62 0x00 > ^ > |__ p > p = ep->fiter_str + 2 (ret1) +2 (ret2) = ep->filter_str + 4 > =============== > so After p[-1] = *(ep->filter_str + 3) = '\0'; > ep->filter_str = 0x61 0x20 0x62 0x00 > > According to ISO C99: " If the output was truncated due to this limit > then the return value is the number of characters (excluding the > terminating null byte) which would have been written to the final > string if enough space had been available" > > The last snprintf will end with 'NULL', so I think p[-1] = '\0' can > also be deleted
Hmm, now that I see it, I think that it is there to remove the last space (even if there shouldn't be any because the last snprintf will be truncated).
Code LGTM as-is, even if puzzling.
CJ
>
|  |