linux-kernel - Re: [PATCH 30/30] sound/soc: Use kmemdup rather than duplicating its implementation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CABXRUiQH2c5knAxnegNc1J2uyqy3OVU=qEorcZkUtMoJhvb_8Q@mail.gmail.com>
Date:   Wed, 3 Jul 2019 22:58:43 +0800
From:   Fuqian Huang <huangfq.daxian@...il.com>
To:     Richard Fitzgerald <rf@...nsource.cirrus.com>
Cc:     Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Jie Yang <yang.jie@...ux.intel.com>,
        patches@...nsource.cirrus.com, alsa-devel@...a-project.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 30/30] sound/soc: Use kmemdup rather than duplicating its implementation

Ok, I will separate them into two patches in the v2 patch set. Also,
there is a typo in the v1(memset should be memcpy).

Richard Fitzgerald <rf@...nsource.cirrus.com> 於 2019年7月3日週三 下午9:55寫道：
>
> On 03/07/19 14:18, Fuqian Huang wrote:
> > kmemdup is introduced to duplicate a region of memory in a neat way.
> > Rather than kmalloc/kzalloc + memset, which the programmer needs to
> > write the size twice (sometimes lead to mistakes), kmemdup improves
> > readability, leads to smaller code and also reduce the chances of mistakes.
> > Suggestion to use kmemdup rather than using kmalloc/kzalloc + memset.
> >
> > Signed-off-by: Fuqian Huang <huangfq.daxian@...il.com>
> > ---
> >   sound/soc/codecs/wm0010.c             | 4 +---
> >   sound/soc/intel/atom/sst/sst_loader.c | 3 +--
>
> Should be one patch per file as the drivers are not related to each
> other at all, and if one needed a revert you couldn't revert this
> patch because it would revert both drivers.
>
> But apart from that, for wm0010.c:
> Acked-by: Richard Fitzgerald <rf@...nsource.cirrus.com>
>
> >   2 files changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/sound/soc/codecs/wm0010.c b/sound/soc/codecs/wm0010.c
> > index 727d6703c905..807826f30f58 100644
> > --- a/sound/soc/codecs/wm0010.c
> > +++ b/sound/soc/codecs/wm0010.c
> > @@ -515,7 +515,7 @@ static int wm0010_stage2_load(struct snd_soc_component *component)
> >       dev_dbg(component->dev, "Downloading %zu byte stage 2 loader\n", fw->size);
> >
> >       /* Copy to local buffer first as vmalloc causes problems for dma */
> > -     img = kzalloc(fw->size, GFP_KERNEL | GFP_DMA);
> > +     img = kmemdup(&fw->data[0], fw->size, GFP_KERNEL | GFP_DMA);
> >       if (!img) {
> >               ret = -ENOMEM;
> >               goto abort2;
> > @@ -527,8 +527,6 @@ static int wm0010_stage2_load(struct snd_soc_component *component)
> >               goto abort1;
> >       }
> >
> > -     memcpy(img, &fw->data[0], fw->size);
> > -
> >       spi_message_init(&m);
> >       memset(&t, 0, sizeof(t));
> >       t.rx_buf = out;
> > diff --git a/sound/soc/intel/atom/sst/sst_loader.c b/sound/soc/intel/atom/sst/sst_loader.c
> > index ce11c36848c4..cc95af35c060 100644
> > --- a/sound/soc/intel/atom/sst/sst_loader.c
> > +++ b/sound/soc/intel/atom/sst/sst_loader.c
> > @@ -288,14 +288,13 @@ static int sst_cache_and_parse_fw(struct intel_sst_drv *sst,
> >   {
> >       int retval = 0;
> >
> > -     sst->fw_in_mem = kzalloc(fw->size, GFP_KERNEL);
> > +     sst->fw_in_mem = kmemdup(fw->data, fw->size, GFP_KERNEL);
> >       if (!sst->fw_in_mem) {
> >               retval = -ENOMEM;
> >               goto end_release;
> >       }
> >       dev_dbg(sst->dev, "copied fw to %p", sst->fw_in_mem);
> >       dev_dbg(sst->dev, "phys: %lx", (unsigned long)virt_to_phys(sst->fw_in_mem));
> > -     memcpy(sst->fw_in_mem, fw->data, fw->size);
> >       retval = sst_parse_fw_memcpy(sst, fw->size, &sst->memcpy_list);
> >       if (retval) {
> >               dev_err(sst->dev, "Failed to parse fw\n");
> >
>