Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 22 Jul 2021 14:36:03 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Dongliang Mu <mudongliangabcd@gmail.com>
Cc:     Vinod Koul <vkoul@kernel.org>,
        Bard Liao <yung-chuan.liao@linux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>,
        Sanyog Kale <sanyog.r.kale@intel.com>,
        alsa-devel@alsa-project.org,
        linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] soundwire: stream: add s_rt into slave_rt_list before
 sdw_config_stream
Message-ID: <20210722113603.GX25548@kadam>
References: <20210722105611.1582803-1-mudongliangabcd@gmail.com>
 <20210722111654.GW25548@kadam>
 <CAD-N9QWjjmUUSOmcBJX2n0zJrNRK3hPd03M=NbAfWNamEkfhJw@mail.gmail.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAD-N9QWjjmUUSOmcBJX2n0zJrNRK3hPd03M=NbAfWNamEkfhJw@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from kadam (102.222.70.252) by JNAP275CA0026.ZAFP275.PROD.OUTLOOK.COM (2603:1086:0:4d::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4352.26 via Frontend Transport; Thu, 22 Jul 2021 11:36:34 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 2f631fcf-9c6b-4d62-5c0f-08d94d04f8ba
X-MS-TrafficTypeDiagnostic: CO1PR10MB4707:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <CO1PR10MB4707061C95776C605D69BB708EE49@CO1PR10MB4707.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:376;
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: N2j1SjbhI5gW7rQVzeuMV9EZLJLqojVY4Vdg4fhYz9iCU1Gaap/Rh2D8yb7R+Jmjxx+DadbBFt+W02ZkQiCJnw3BvO8kiJoPw7Eamr0h1pEc7bwWksmYPGHjyi+oE++eiVLsseH/ZgCSoYByZ5nKZ+yN0Fd06/qqw2Qs9Kjp6xeKl9JEtg9tDIiunDrNn+tiKZLkbDzfCOChy5mYEt+Dwg0wqpzfTzSBWagi8LpI8KYYBh2EcH9byWLE4BbFDVt6lWxWizN3QDiwfdwXSezGipluCZHsjQSFGXX8qw2QmNBBdaLN0Wnn6XMW+Fq01Mqg5/K9ccWRSC+h1MSC+TUp1ILJ6PuE7chXyDK2gi40PYmIwpt/r17DER23Qyr8w5HMNlm2XwM/YaJo29L6oZbxndMwfRfwvU0diefhDdTm5H/pGV37/kyviMDcxR0z+isuAs5zFhVOE7IpKpTJnAIwQWOsOA7tike9bSmFPqZ97YFyNDZ9YC9OiJELYfnXkTCMMabPponSXPBKoJ/KlcpbbqR8l0x8xgJamYZ44TbIwkevoO2xnaiVC44JJCE2NsOLriuMMh5UVUx9ZbByrhaAGUNW37bP4Z3iUQzoTeoqOPNrY5mpoaLHrCGtHSkC8pT/TaREI0PPMmmbJDEabX98wu+ikcM6C4wCbiMyPSA6Cm7imz6aiBk9Uzbkwdn0BeEnHsl3TBDIDuSwQi1J6wyFVA==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MWHPR1001MB2365.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(136003)(366004)(396003)(39860400002)(346002)(376002)(956004)(38100700002)(478600001)(6916009)(26005)(4326008)(86362001)(54906003)(6496006)(8936002)(33656002)(38350700002)(52116002)(2906002)(44832011)(1076003)(66946007)(316002)(8676002)(66476007)(66556008)(9686003)(186003)(6666004)(53546011)(33716001)(9576002)(55016002)(83380400001)(5660300002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EIxHv9/tZvSw6CtyCK/c7lrdnvwEgMecfU1D6xzVEUrQk9RQvazL+F2ZRxOb?=
 =?us-ascii?Q?s2WJGBZftjtdam7IoNRQ41hRCxJmTuONX5+vqEZNfWashQh1w1R6KVSP143f?=
 =?us-ascii?Q?tS+mwWX+7GjwlBQQU7oRfllzRoBBc4yueGwBH8OqhHR4F8iiaoa95THhl2GN?=
 =?us-ascii?Q?RBNQxUoobLWpAIpui1oKn5ZHK/a2OeiIiNrByA32mEPHHPIib+7SSPSGbVZA?=
 =?us-ascii?Q?+ifQyAF8LEfDEB2NkQRsBbDyMavxiDvmu2+t9YEAvPrgUR/V1CnhVgeMOS30?=
 =?us-ascii?Q?+328//Yo2NgCxZ2Mjvxeo/loVYCkGMXJgWqP3rHEc3HfrdkjCzM8BxBqQyTr?=
 =?us-ascii?Q?vsguhiI2V7yDz3M3beHr4816d9B8sy+6yZhVr+8TqKblGeAUKfCvWYhBKHWL?=
 =?us-ascii?Q?0lax08inVEYz/HxjxUNSsaWkMzn6b4bkQgJHqf+jarPK32MzlZdlOlV/ZRka?=
 =?us-ascii?Q?x/z/vXJVqXs4mLRKnthiCr5xBPCvqR51ABI27iiLsvYwGEkwpRlnZi7D0pFH?=
 =?us-ascii?Q?Gzg46Gtjr0RUGoIqEs4rGBs7JRJtNWvUN0o5CR2DQ4KpTaRZYIhfHIlgX6Qk?=
 =?us-ascii?Q?aEFjtU8IkLWBz0dIMBBd/H3oMzCoKdSdD9+XfnaYnYBni9WkQmk1Q26Pk0P2?=
 =?us-ascii?Q?/n0V/dyPhp1oVaUIFLaxsPKn0vbz2yZkDR5gRwwUGQr6GaGofgAalA1PDHc1?=
 =?us-ascii?Q?wVUk5DJkj0JA3MRWAPWGvxjlNgjLw4ENFAHjSiiqXDn+6nzvWKMdzGvpnwzD?=
 =?us-ascii?Q?/NlLg84R1S9k9Mln9ZkXLK01u6ZMeY2nCwjpt5lVo2Dq1u+31et0rzjA+0Q6?=
 =?us-ascii?Q?qB98r9RtL85U21W3TRnpAWzdmbbkj7A5AdqB4p/t2ckDyulbBQcnfTgZTrQM?=
 =?us-ascii?Q?UHeyB0EIMWjwO+gSEkIiw+240PY8tiMvx3Ow8j41rydwyyBZGpc8+7dn/kau?=
 =?us-ascii?Q?1gxbaH0HAq8ZJlJxxOkRqP82QxAwBEVLFSTQ2ODGK0E+ET9n8NQ55r7IOsNn?=
 =?us-ascii?Q?MueFexHAfgAvEWP7IaXPPQTwjOlrSqpRoJtSqJrc9o+qmCtPUOOr8VbJboF5?=
 =?us-ascii?Q?XI79hunHcTZk4UGa7o/Qg4DM2UyPBE3Qob/bxCVKoM4/m814mmHCU3W2uLEF?=
 =?us-ascii?Q?3lGmqfVhlO9OtsxYItJh5+EBJM4QFox6rSjIu1HDSzOUgZJAkQmB+9czyAQV?=
 =?us-ascii?Q?XKHNABk3iBAKEMvR7QpBu53vu0ysDbcn24vExSqGYt6ZxaaU//inp0WmFMbv?=
 =?us-ascii?Q?z+R0fZvTzsUKIbCw2UZW1WxM1gtpPQ31zrC0mBkhg928btep0I3AVcVCzAwQ?=
 =?us-ascii?Q?VMlLq2KZvgMp+KGP7WB5V0ws?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2f631fcf-9c6b-4d62-5c0f-08d94d04f8ba
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Jul 2021 11:36:40.2393
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: L/lcNAZmOb0yc/NNOB3+Nwn4QBxG3hkQm6oftUsUiMdfOP33JxcwDMRy+WsymfvR+xLhgaq/apdu1hzJN3BjB3FWT81TwOYXmrnbe3l8EVQ=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR10MB4707
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10052 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 mlxscore=0 malwarescore=0
 adultscore=0 phishscore=0 suspectscore=0 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2104190000
 definitions=main-2107220077
X-Proofpoint-GUID: 4kX6Daw5wUUG9ge1Liy49FAKPLI57DWx
X-Proofpoint-ORIG-GUID: 4kX6Daw5wUUG9ge1Liy49FAKPLI57DWx
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Jul 22, 2021 at 07:24:16PM +0800, Dongliang Mu wrote:
> On Thu, Jul 22, 2021 at 7:17 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > On Thu, Jul 22, 2021 at 06:56:11PM +0800, Dongliang Mu wrote:
> > > The commit 48f17f96a817 ("soundwire: stream: fix memory leak in stream
> > > config error path") fixes the memory leak by implicitly freeing the s_rt
> > > object. However, this fixing style is not very good.
> > >
> > > The better fix is to move list_add_tail before sdw_config_stream and
> > > revert the previous commit.
> > >
> > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com>
> > > ---
> > >  drivers/soundwire/stream.c | 14 +++-----------
> > >  1 file changed, 3 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> > > index 1a18308f4ef4..66a4ce4f923f 100644
> > > --- a/drivers/soundwire/stream.c
> > > +++ b/drivers/soundwire/stream.c
> > > @@ -1373,19 +1373,11 @@ int sdw_stream_add_slave(struct sdw_slave *slave,
> > >               goto stream_error;
> > >       }
> > >
> > > +     list_add_tail(&s_rt->m_rt_node, &m_rt->slave_rt_list);
> > > +
> > >       ret = sdw_config_stream(&slave->dev, stream, stream_config, true);
> >
> > There some sanity checks on the stream inside sdw_config_stream() so
> > that's probably why we didn't add it until later.  (I don't know the
> > code well, but that's what I would suspect from a glance).
> 
> I think those sanity checks are not much related to list_add_tail.

Yeah.  I can't point to a bug that it causes, it just feels ugly to add
invalid data to the list.

You didn't add Rander to the CC list and he may have had a good reason
to do it that way.

regards,
dan carpenter