1. Patchew
  2. linux
  3. View series

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

Markus Elfring posted 1 patch 3 months, 1 week ago 
arch/sparc/kernel/time_64.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
[PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()
Posted by Markus Elfring 3 months, 1 week ago 
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()
Posted by Geert Uytterhoeven 3 months, 1 week ago 
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()
Posted by Markus Elfring 3 months, 1 week ago 
…>> +++ 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()
Posted by Geert Uytterhoeven 3 months, 1 week ago 
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()
Posted by Dan Carpenter 3 months, 1 week ago 
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
Re: [PATCH] sparc: time: Use pointer from memcpy() call for assignment in setup_sparc64_timer()
Posted by David Laight 3 months, 1 week ago 
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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.