* [PATCH] ALSA: emu10k1: Refactor copy_to_user() usage in snd_emu10k1_fx8010_ioctl()
@ 2024-09-15 14:24 Markus Elfring
2024-09-16 7:20 ` Takashi Iwai
0 siblings, 1 reply; 3+ messages in thread
From: Markus Elfring @ 2024-09-15 14:24 UTC (permalink / raw)
To: linux-sound, Jaroslav Kysela, Takashi Iwai; +Cc: Julia Lawall
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 Sep 2024 16:16:45 +0200
Assign values from selected expressions where copy_to_user() calls
are involved to additional local variables so that the number of
kfree() calls can be reduced accordingly.
This issue was transformed by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
sound/pci/emu10k1/emufx.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
index 03efc317e05f..2f20878a78c9 100644
--- a/sound/pci/emu10k1/emufx.c
+++ b/sound/pci/emu10k1/emufx.c
@@ -2496,12 +2496,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
if (!info)
return -ENOMEM;
snd_emu10k1_fx8010_info(emu, info);
- if (copy_to_user(argp, info, sizeof(*info))) {
+
+ {
+ unsigned long ctu = copy_to_user(argp, info, sizeof(*info));
kfree(info);
- return -EFAULT;
+ return ctu ? -EFAULT : 0;
}
- kfree(info);
- return 0;
case SNDRV_EMU10K1_IOCTL_CODE_POKE:
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
@@ -2517,12 +2517,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
if (IS_ERR(icode))
return PTR_ERR(icode);
res = snd_emu10k1_icode_peek(emu, icode);
- if (res == 0 && copy_to_user(argp, icode, sizeof(*icode))) {
+
+ {
+ bool ctu = res == 0 && copy_to_user(argp, icode, sizeof(*icode));
kfree(icode);
- return -EFAULT;
+ return ctu ? -EFAULT : res;
}
- kfree(icode);
- return res;
case SNDRV_EMU10K1_IOCTL_PCM_POKE:
ipcm = memdup_user(argp, sizeof(*ipcm));
if (IS_ERR(ipcm))
@@ -2535,12 +2535,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
if (IS_ERR(ipcm))
return PTR_ERR(ipcm);
res = snd_emu10k1_ipcm_peek(emu, ipcm);
- if (res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm))) {
+
+ {
+ bool ctu = res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm));
kfree(ipcm);
- return -EFAULT;
+ return ctu ? -EFAULT : res;
}
- kfree(ipcm);
- return res;
case SNDRV_EMU10K1_IOCTL_TRAM_SETUP:
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
--
2.46.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] ALSA: emu10k1: Refactor copy_to_user() usage in snd_emu10k1_fx8010_ioctl()
2024-09-15 14:24 [PATCH] ALSA: emu10k1: Refactor copy_to_user() usage in snd_emu10k1_fx8010_ioctl() Markus Elfring
@ 2024-09-16 7:20 ` Takashi Iwai
2024-09-16 7:33 ` Markus Elfring
0 siblings, 1 reply; 3+ messages in thread
From: Takashi Iwai @ 2024-09-16 7:20 UTC (permalink / raw)
To: Markus Elfring; +Cc: linux-sound, Jaroslav Kysela, Takashi Iwai, Julia Lawall
On Sun, 15 Sep 2024 16:24:25 +0200,
Markus Elfring wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 15 Sep 2024 16:16:45 +0200
>
> Assign values from selected expressions where copy_to_user() calls
> are involved to additional local variables so that the number of
> kfree() calls can be reduced accordingly.
>
> This issue was transformed by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
This doesn't look like an improvement. With the refactoring, it's
rather harder to read.
thanks,
Takashi
> ---
> sound/pci/emu10k1/emufx.c | 24 ++++++++++++------------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
> index 03efc317e05f..2f20878a78c9 100644
> --- a/sound/pci/emu10k1/emufx.c
> +++ b/sound/pci/emu10k1/emufx.c
> @@ -2496,12 +2496,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
> if (!info)
> return -ENOMEM;
> snd_emu10k1_fx8010_info(emu, info);
> - if (copy_to_user(argp, info, sizeof(*info))) {
> +
> + {
> + unsigned long ctu = copy_to_user(argp, info, sizeof(*info));
> kfree(info);
> - return -EFAULT;
> + return ctu ? -EFAULT : 0;
> }
> - kfree(info);
> - return 0;
> case SNDRV_EMU10K1_IOCTL_CODE_POKE:
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> @@ -2517,12 +2517,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
> if (IS_ERR(icode))
> return PTR_ERR(icode);
> res = snd_emu10k1_icode_peek(emu, icode);
> - if (res == 0 && copy_to_user(argp, icode, sizeof(*icode))) {
> +
> + {
> + bool ctu = res == 0 && copy_to_user(argp, icode, sizeof(*icode));
> kfree(icode);
> - return -EFAULT;
> + return ctu ? -EFAULT : res;
> }
> - kfree(icode);
> - return res;
> case SNDRV_EMU10K1_IOCTL_PCM_POKE:
> ipcm = memdup_user(argp, sizeof(*ipcm));
> if (IS_ERR(ipcm))
> @@ -2535,12 +2535,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
> if (IS_ERR(ipcm))
> return PTR_ERR(ipcm);
> res = snd_emu10k1_ipcm_peek(emu, ipcm);
> - if (res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm))) {
> +
> + {
> + bool ctu = res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm));
> kfree(ipcm);
> - return -EFAULT;
> + return ctu ? -EFAULT : res;
> }
> - kfree(ipcm);
> - return res;
> case SNDRV_EMU10K1_IOCTL_TRAM_SETUP:
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> --
> 2.46.0
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ALSA: emu10k1: Refactor copy_to_user() usage in snd_emu10k1_fx8010_ioctl()
2024-09-16 7:20 ` Takashi Iwai
@ 2024-09-16 7:33 ` Markus Elfring
0 siblings, 0 replies; 3+ messages in thread
From: Markus Elfring @ 2024-09-16 7:33 UTC (permalink / raw)
To: Takashi Iwai, linux-sound
Cc: Takashi Iwai, LKML, Jaroslav Kysela, Julia Lawall
>> From: Markus Elfring <elfring@users.sourceforge.net>
>> Date: Sun, 15 Sep 2024 16:16:45 +0200
>>
>> Assign values from selected expressions where copy_to_user() calls
>> are involved to additional local variables so that the number of
>> kfree() calls can be reduced accordingly.
>>
>> This issue was transformed by using the Coccinelle software.
>>
>> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
>
> This doesn't look like an improvement. With the refactoring, it's
> rather harder to read.
…
>> ---
>> sound/pci/emu10k1/emufx.c | 24 ++++++++++++------------
>> 1 file changed, 12 insertions(+), 12 deletions(-)
>>
>> diff --git a/sound/pci/emu10k1/emufx.c b/sound/pci/emu10k1/emufx.c
>> index 03efc317e05f..2f20878a78c9 100644
>> --- a/sound/pci/emu10k1/emufx.c
>> +++ b/sound/pci/emu10k1/emufx.c
>> @@ -2496,12 +2496,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
>> if (!info)
>> return -ENOMEM;
>> snd_emu10k1_fx8010_info(emu, info);
>> - if (copy_to_user(argp, info, sizeof(*info))) {
>> +
>> + {
>> + unsigned long ctu = copy_to_user(argp, info, sizeof(*info));
>> kfree(info);
>> - return -EFAULT;
>> + return ctu ? -EFAULT : 0;
>> }
>> - kfree(info);
>> - return 0;
>> case SNDRV_EMU10K1_IOCTL_CODE_POKE:
>> if (!capable(CAP_SYS_ADMIN))
>> return -EPERM;
>> @@ -2517,12 +2517,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
>> if (IS_ERR(icode))
>> return PTR_ERR(icode);
>> res = snd_emu10k1_icode_peek(emu, icode);
>> - if (res == 0 && copy_to_user(argp, icode, sizeof(*icode))) {
>> +
>> + {
>> + bool ctu = res == 0 && copy_to_user(argp, icode, sizeof(*icode));
>> kfree(icode);
>> - return -EFAULT;
>> + return ctu ? -EFAULT : res;
>> }
>> - kfree(icode);
>> - return res;
>> case SNDRV_EMU10K1_IOCTL_PCM_POKE:
>> ipcm = memdup_user(argp, sizeof(*ipcm));
>> if (IS_ERR(ipcm))
>> @@ -2535,12 +2535,12 @@ static int snd_emu10k1_fx8010_ioctl(struct snd_hwdep * hw, struct file *file, un
>> if (IS_ERR(ipcm))
>> return PTR_ERR(ipcm);
>> res = snd_emu10k1_ipcm_peek(emu, ipcm);
>> - if (res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm))) {
>> +
>> + {
>> + bool ctu = res == 0 && copy_to_user(argp, ipcm, sizeof(*ipcm));
>> kfree(ipcm);
>> - return -EFAULT;
>> + return ctu ? -EFAULT : res;
>> }
>> - kfree(ipcm);
>> - return res;
>> case SNDRV_EMU10K1_IOCTL_TRAM_SETUP:
>> if (!capable(CAP_SYS_ADMIN))
>> return -EPERM;
>> --
>> 2.46.0
>>
* Can three less kfree() calls matter here finally?
* Do you find the affected control flow really “too succinct” now?
Regards,
Markus
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-09-16 7:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-15 14:24 [PATCH] ALSA: emu10k1: Refactor copy_to_user() usage in snd_emu10k1_fx8010_ioctl() Markus Elfring
2024-09-16 7:20 ` Takashi Iwai
2024-09-16 7:33 ` Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox