[PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 31 Oct 2025 08:36:13 +0100

A pointer was assigned to a variable. The same pointer was used for
the destination parameter of a memcpy() call.
This function is documented in the way that the same value is returned.
Thus convert two separate statements into a direct variable assignment for
the return value from a memory copy action.

The source code was transformed by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 arch/sparc/kernel/time_64.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/sparc/kernel/time_64.c b/arch/sparc/kernel/time_64.c
index b32f27f929d1..e9c29574cd59 100644
--- a/arch/sparc/kernel/time_64.c
+++ b/arch/sparc/kernel/time_64.c
@@ -760,9 +760,7 @@ void setup_sparc64_timer(void)
 			     : /* no outputs */
 			     : "r" (pstate));
 
-	sevt = this_cpu_ptr(&sparc64_events);
-
-	memcpy(sevt, &sparc64_clockevent, sizeof(*sevt));
+	sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent, sizeof(*sevt));
 	sevt->cpumask = cpumask_of(smp_processor_id());
 
 	clockevents_register_device(sevt);
-- 
2.51.1

Re: [PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[Geert Uytterhoeven]

Hi Markus,

On Fri, 31 Oct 2025 at 08:46, Markus Elfring <Markus.Elfring@web.de> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 31 Oct 2025 08:36:13 +0100
>
> A pointer was assigned to a variable. The same pointer was used for
> the destination parameter of a memcpy() call.
> This function is documented in the way that the same value is returned.
> Thus convert two separate statements into a direct variable assignment for
> the return value from a memory copy action.
>
> The source code was transformed by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Thanks for your patch!

> --- a/arch/sparc/kernel/time_64.c
> +++ b/arch/sparc/kernel/time_64.c
> @@ -760,9 +760,7 @@ void setup_sparc64_timer(void)
>                              : /* no outputs */
>                              : "r" (pstate));
>
> -       sevt = this_cpu_ptr(&sparc64_events);
> -
> -       memcpy(sevt, &sparc64_clockevent, sizeof(*sevt));
> +       sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent, sizeof(*sevt));

IMHO this makes the code harder to read:
  - Only 0.15% of the memcpy() calls in the kernel use the
    memcpy() chaining feature,
  - The line is now longer than the 80-character limit, which is still
    customary for this file.

>         sevt->cpumask = cpumask_of(smp_processor_id());
>
>         clockevents_register_device(sevt);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[Markus Elfring]

…>> +++ b/arch/sparc/kernel/time_64.c
>> @@ -760,9 +760,7 @@ void setup_sparc64_timer(void)
>>                              : /* no outputs */
>>                              : "r" (pstate));
>>
>> -       sevt = this_cpu_ptr(&sparc64_events);
>> -
>> -       memcpy(sevt, &sparc64_clockevent, sizeof(*sevt));
>> +       sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent, sizeof(*sevt));
> 
> IMHO this makes the code harder to read:
>   - Only 0.15% of the memcpy() calls in the kernel use the
>     memcpy() chaining feature,

I obviously propose to refactor this implementation detail.


>   - The line is now longer than the 80-character limit, which is still
>     customary for this file.

Would you like to get a subsequent patch variant?

Regards,
Markus

Re: [PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[David Laight]

On Fri, 31 Oct 2025 09:46:25 +0100
Markus Elfring <Markus.Elfring@web.de> wrote:

> …>> +++ b/arch/sparc/kernel/time_64.c
> >> @@ -760,9 +760,7 @@ void setup_sparc64_timer(void)
> >>                              : /* no outputs */
> >>                              : "r" (pstate));
> >>
> >> -       sevt = this_cpu_ptr(&sparc64_events);
> >> -
> >> -       memcpy(sevt, &sparc64_clockevent, sizeof(*sevt));
> >> +       sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent, sizeof(*sevt));  
> > 
> > IMHO this makes the code harder to read:
> >   - Only 0.15% of the memcpy() calls in the kernel use the
> >     memcpy() chaining feature,  
> 
> I obviously propose to refactor this implementation detail.

Reduce it to 0% and the kernel memcpy() can be made 'void'. :-)
That will simplify the architecture specific implementations.

The same can be done for strcpy() and strcat() (etc).
Where the 'useful' return value would be the address of the '\0'.

	David

Re: [PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[Geert Uytterhoeven]

Hi Markus,

On Fri, 31 Oct 2025 at 09:46, Markus Elfring <Markus.Elfring@web.de> wrote:
> …>> +++ b/arch/sparc/kernel/time_64.c
> >> @@ -760,9 +760,7 @@ void setup_sparc64_timer(void)
> >>                              : /* no outputs */
> >>                              : "r" (pstate));
> >>
> >> -       sevt = this_cpu_ptr(&sparc64_events);
> >> -
> >> -       memcpy(sevt, &sparc64_clockevent, sizeof(*sevt));
> >> +       sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent, sizeof(*sevt));
> >
> > IMHO this makes the code harder to read:
> >   - Only 0.15% of the memcpy() calls in the kernel use the
> >     memcpy() chaining feature,

It is also less clear the passed size matches the destination pointer.

> I obviously propose to refactor this implementation detail.

Oh no...

<other-bad-ideas>

The above function could be shortened by writing

    (sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent,
sizeof(*sevt)))->cpumask = cpumask_of(smp_processor_id());

And after introducing a variant of clockevents_register_device() that
takes the cpumask as a parameter:

        clockevents_register_device_with_cpumask(memcpy(this_cpu_ptr(&sparc64_events),
&sparc64_clockevent, sizeof(*sevt)), cpumask_of(smp_processor_id()));

</other-bad-ideas>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()

[Dan Carpenter]

On Fri, Oct 31, 2025 at 11:08:39AM +0100, Geert Uytterhoeven wrote:
> 
> The above function could be shortened by writing
> 
>     (sevt = memcpy(this_cpu_ptr(&sparc64_events), &sparc64_clockevent,
> sizeof(*sevt)))->cpumask = cpumask_of(smp_processor_id());

Heh.

regards,
dan carpenter