lkml.org 
[lkml]   [2023]   [Jan]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] tracing/eprobe: Replace kzalloc with kmalloc
From
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

>

\
 
 \ /
  Last update: 2023-03-26 23:31    [W:0.033 / U:3.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site