From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A56C72F3C30 for ; Sun, 4 Jan 2026 21:42:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.168.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767562938; cv=none; b=s5jRknVIDZmVsqTHjbcRfTCwRHF9DHQ+dq/KIQd/e2rC8TXmyWMwKOvpf71dsFPggqbq2ZcJoHjWAaZXnW95rZ2GUU8DwD5vKKR3ty5muZN5yThRLwGLKkSQYkPgKVsu8pzhqP4iVKit/bLKzlOGCxRaAgLniMD/vkeYcVNjdVc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1767562938; c=relaxed/simple; bh=w16TMO27UtiBI3a5qt3dM5OuRXpQmCG+fP+Ljm2c6Rk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=VokkP+0pl+Jw8VEpqyUs1Esc7f+8DQ4c4AOmpRt18shcycO8VaGE3/REwTGDGdPZHC2oxXarb80LvYMyVZPFBQwaEISKcZZOUwUAsmder8N+Rf3qi0k05WUqrQKIgYNuRUTR0XzrpYXTDS1N2oSq2BRAZNGXEBQr/6mSK1iUqP4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=bcbTp/Yj; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=DT4i9glH; arc=none smtp.client-ip=205.220.168.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="bcbTp/Yj"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="DT4i9glH" Received: from pps.filterd (m0279867.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 604LMIoQ3339686 for ; Sun, 4 Jan 2026 21:42:13 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= rImH/2KlyvyZhocXv4tvAEkdDHKv39O59A2fFmYnOfE=; b=bcbTp/YjDkqO8Wub XcfrHgNSPDqCkudhNfT1jeejUvTPW2FRadXFWrpK22E6VO/cjWLU/dQtuEL5uLYG FeV/22K3iTwmkXxSQ3IyE7j24lfJUZqu3NGMQTpZ+Dd5IMN0l4k4cVQI3sby1FpJ VWOZqYOKoJtQNJg4D2+Mfc5mO2/QxARuipPKVyf8WXDZciyvOaJhCC3QnZRpsd+f Oee3UErhgEMN04n+jjRl0ucUW0r7mI++/ziL+D5c3TzvBfcudKnhvqamLakbC0RF Q0JoQrJ0+d5CP1ZcCWRpJSxikAaeuLZ3jwwHtB2AMf6vU6iXauyGiVQG/otGlKHV ChQSsw== Received: from mail-pl1-f197.google.com (mail-pl1-f197.google.com [209.85.214.197]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4bertcjsm3-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Sun, 04 Jan 2026 21:42:13 +0000 (GMT) Received: by mail-pl1-f197.google.com with SMTP id d9443c01a7336-29f2b45ecffso227563905ad.2 for ; Sun, 04 Jan 2026 13:42:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1767562932; x=1768167732; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=rImH/2KlyvyZhocXv4tvAEkdDHKv39O59A2fFmYnOfE=; b=DT4i9glHQwxVkxk0KyYUQpOVg0mmdF2hyMsudP1BU3LsZSTCZF7l9JEyj49L6jjbh+ yxps/MbN/rN4Pk3cgpwMFv9HSYkaXSidDn9EdGR39C/EBDjgDSmRS943jJ33z8298u85 NeCOmGYPmOunEfopLLDnofkM0/27G9nuOkB0SHZMobW7D79JECBjvaanBPMhN8wtouo5 YxWJNZoSiuk/6Rao5pp8x0C2DzGNVJgN/6zqLHrS4KPKaB8XAPjKtB19THeL1ogJKeT4 crQIoJ8B2vhOURvE3st3+AHHx1BdcAOYV3Os4BvWvo3F1/D91eeBJW/8oso4OiB7qfY4 /maA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1767562932; x=1768167732; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=rImH/2KlyvyZhocXv4tvAEkdDHKv39O59A2fFmYnOfE=; b=fleC5316W6NjO702o1BPt38Zoi48IZLfByqu1s+BgN6h3QK4OjlCgP12KVJqTbJxST rsBp1sI9fLCv2XZuf0zOthtZRyL7nFwUAOAFn78bQusIMgO/38qpJ9vWsFhEn2NeQA2d SMT2fe42sW87emK6AyHrGNh807fHn9Xa1wHdhLTZlgvVWmG/Bk1uT46JZ+LD2DhYq2XK XacVKetyiddI+RSDA2x6nx+WNWBqNJAGcZqJzSLxSovu9z0awTzM5p+F46t33dYkFJXc 6N8qLR18VVhSsYcJ9eXSmOZ0kHbpCsrd8Xd4gjLJRL4cA7pEISjmYIjRVqS8wlX8IXOu K5oQ== X-Forwarded-Encrypted: i=1; AJvYcCV+lfRCgI1NEIpXKSgektWRQ43YRJRzf1KNPvq1y/hZbvN2q5KZhJxQjxguRdVpmaLLhTUsnwPfT3Hdgqg=@vger.kernel.org X-Gm-Message-State: AOJu0YwM6lpAChWyoeWDN2JL3ZKIz7RQp3ql8DCPEQW5ZSLuZPzbRrJ6 c1PtJScwrd4ZcZ/VHxhNB528dBJd0PW3/dEtXD32SO8+RXfh2pr6J6LA0TH1164xPvb4HsXejjP JteMdDmEvBOJIB67j+jMg8BYoJQuNa0ryhHmiHmyYGxryYWegq0AqMKBJ8nmVLV7EJsbl0UKZHw == X-Gm-Gg: AY/fxX40+9/eYjjKCezwq6wJoAHNDeinTMq5kl7HnwleVl2UQZzaosLfRQ0OPWDDy+9 IYu3/GXmlovPCPwU2dzUt1eQyC90vte51/CvXcTrALuSJFo0P2sLzv6X+Yiof1/huutXuqtUTC0 4LUZNB3iNovgKGqGorL5Egb07yADIF7SG1WXet8xMQ9MXudFxnCSpCK8Bm/ZePHDjKsSttHKPd9 6jONab7OgATuJaM8xMeApppe77D4rieKVYtVWaF9bUiduhuNsmOjFIj2IzPFRkNt2/tPYwQgdd7 Oz5asNw8fCsA9BJVtmlC4XM4Az3X+JFOAv+R7WPe5SyjqssmrUDyEa1LRF3EMiEhfQe5lBudd3R iIXpMGoTu3i4kHxAKl/TYSVMg2fSTMeO1vhIgNIVAJbEEtm6ycanEeIzIkDDC+MPHPrENGr6Qp9 JUtKUK4p4= X-Received: by 2002:a17:903:244f:b0:2a1:3ade:c351 with SMTP id d9443c01a7336-2a2f222bd48mr542175295ad.2.1767562932405; Sun, 04 Jan 2026 13:42:12 -0800 (PST) X-Google-Smtp-Source: AGHT+IE6mzsu28xlvCSCcsYwKe8FITpxti/Q0k/fto0O4UNvWtGYcXQhJtAxHgDry153WPUw0oTW/w== X-Received: by 2002:a17:903:244f:b0:2a1:3ade:c351 with SMTP id d9443c01a7336-2a2f222bd48mr542175165ad.2.1767562931937; Sun, 04 Jan 2026 13:42:11 -0800 (PST) Received: from [192.168.0.74] (n1-41-240-65.bla22.nsw.optusnet.com.au. [1.41.240.65]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2a322d76a19sm379338355ad.101.2026.01.04.13.42.09 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sun, 04 Jan 2026 13:42:11 -0800 (PST) Message-ID: Date: Mon, 5 Jan 2026 08:42:06 +1100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 1/3] tee: qcomtee: call: Fix confusing cleanup.h syntax To: Jens Wiklander Cc: Sumit Garg , Krzysztof Kozlowski , linux-arm-msm@vger.kernel.org, op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org References: <20251208020844.5310-4-krzysztof.kozlowski@oss.qualcomm.com> <7b074ee0-4f10-4fba-9680-3d87dcf766c1@oss.qualcomm.com> Content-Language: en-US From: Amirreza Zarrabi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Proofpoint-ORIG-GUID: HHhi6NhsfG9Oxv1tMo9ZitLvJ-LrNjEm X-Authority-Analysis: v=2.4 cv=VNbQXtPX c=1 sm=1 tr=0 ts=695adeb5 cx=c_pps a=cmESyDAEBpBGqyK7t0alAg==:117 a=hi51d+lTLNy/RbqRqnOomQ==:17 a=IkcTkHD0fZMA:10 a=vUbySO9Y5rIA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=EUspDBNiAAAA:8 a=QEsY0_0xN9MvHtGfP8gA:9 a=3ZKOabzyN94A:10 a=QEXdDO2ut3YA:10 a=1OuFwYUASf3TG4hYMiVC:22 X-Proofpoint-GUID: HHhi6NhsfG9Oxv1tMo9ZitLvJ-LrNjEm X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwMTA0MDIwMCBTYWx0ZWRfXwjq5pCSAbY+6 FiFmYJl/W1qt1q1+uurbq2NDuGrtkBBdieXHje2/QUpy2gjyBnvXX/+seLAb6dRRm6lutmO5q82 cLAwv3lYoQ71PJf+X3L8d4vqKVWTm/tXGCKIutXR4AkTmE5/vFdKbzakM4Y7hnaBytb3GRzlEzu oS4XuDMUSG4HCiJFK9IWVf36Go18h7Zv8GVec7NUxOkxGCUc+EKilJ4ZDZaFrZTCGXmbNUXAji4 cHhRrOqwg7dTvikab55DLPcIkYulB9wOPl7FP9WEK27l3lPL3oCLiUTphDXY7Q1gOHMajXBC38e FkkuLQkZ8QkvRy/JmAy1msT3AbjEDvOb2ukjzqKy6ka2fmV6eAq1o3C6wH6fnMqk1LJBlcbk10G L9T8AXZYBkJL0V2VjLuCklvl75VmBI4nAUug5A6liRX5AqUEKxbSISbmyv7ZFL/A+AXf/M/Pj2M CQrXXMVKhZNNgB/OTEw== X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1121,Hydra:6.1.9,FMLib:17.12.100.49 definitions=2026-01-04_06,2025-12-31_01,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 suspectscore=0 impostorscore=0 priorityscore=1501 adultscore=0 phishscore=0 bulkscore=0 malwarescore=0 lowpriorityscore=0 spamscore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2512120000 definitions=main-2601040200 Hi, On 12/16/2025 6:33 PM, Jens Wiklander wrote: > Hi, > > On Mon, Dec 15, 2025 at 9:30 PM Amirreza Zarrabi > wrote: >> >> Hi, >> >> On 12/12/2025 12:39 PM, Sumit Garg wrote: >>> On Fri, Dec 12, 2025 at 02:07:40AM +0100, Krzysztof Kozlowski wrote: >>>> On 12/12/2025 01:55, Sumit Garg wrote: >>>>> On Mon, Dec 08, 2025 at 03:08:45AM +0100, Krzysztof Kozlowski wrote: >>>>>> Initializing automatic __free variables to NULL without need (e.g. >>>>>> branches with different allocations), followed by actual allocation is >>>>>> in contrary to explicit coding rules guiding cleanup.h: >>>>>> >>>>>> "Given that the "__free(...) = NULL" pattern for variables defined at >>>>>> the top of the function poses this potential interdependency problem the >>>>>> recommendation is to always define and assign variables in one statement >>>>>> and not group variable definitions at the top of the function when >>>>>> __free() is used." >>>>>> >>>>>> Code does not have a bug, but is less readable and uses discouraged >>>>>> coding practice, so fix that by moving declaration to the place of >>>>>> assignment. >>>>> >>>>> Okay I see but.. >>>>> >>>>>> >>>>>> Signed-off-by: Krzysztof Kozlowski >>>>>> --- >>>>>> drivers/tee/qcomtee/call.c | 17 ++++++++--------- >>>>>> 1 file changed, 8 insertions(+), 9 deletions(-) >>>>>> >>>>>> diff --git a/drivers/tee/qcomtee/call.c b/drivers/tee/qcomtee/call.c >>>>>> index 65f9140d4e1f..8f8830f0df26 100644 >>>>>> --- a/drivers/tee/qcomtee/call.c >>>>>> +++ b/drivers/tee/qcomtee/call.c >>>>>> @@ -395,9 +395,7 @@ static int qcomtee_object_invoke(struct tee_context *ctx, >>>>>> struct tee_ioctl_object_invoke_arg *arg, >>>>>> struct tee_param *params) >>>>>> { >>>>>> - struct qcomtee_object_invoke_ctx *oic __free(kfree) = NULL; >>>>>> struct qcomtee_context_data *ctxdata = ctx->data; >>>>>> - struct qcomtee_arg *u __free(kfree) = NULL; >>>>>> struct qcomtee_object *object; >>>>>> int i, ret, result; >>>>>> >>>>>> @@ -412,12 +410,14 @@ static int qcomtee_object_invoke(struct tee_context *ctx, >>>>>> } >>>>>> >>>>>> /* Otherwise, invoke a QTEE object: */ >>>>>> - oic = qcomtee_object_invoke_ctx_alloc(ctx); >>>>>> + struct qcomtee_object_invoke_ctx *oic __free(kfree) = >>>>>> + qcomtee_object_invoke_ctx_alloc(ctx); >>>>>> if (!oic) >>>>>> return -ENOMEM; >>>>>> >>>>>> /* +1 for ending QCOMTEE_ARG_TYPE_INV. */ >>>>>> - u = kcalloc(arg->num_params + 1, sizeof(*u), GFP_KERNEL); >>>>>> + struct qcomtee_arg *u __free(kfree) = kcalloc(arg->num_params + 1, sizeof(*u), >>>>>> + GFP_KERNEL); >>>>> >>>>> ..this makes the code less readable with variable declarations floating >>>> >>>> Which is intentional. >>>> >>>>> within the function. I would rather favor to not use the cleanup.h construct >>>>> but use explicit kfree() invocations instead like it's done in all other >>>>> allocations in the TEE subsystem. >>>> >>>> Sure, fair. I just don't get why introducing cleanup.h without actually >>>> accepting its explicitly documented style... >>>> >>> >>> TBH, it is likely overlooked during review of the QTEE driver. Having a >>> builtin warning for the undesired syntax would help the reviewers here. >>> >>> -Sumit >> >> While the style may seem unusual -- as stated in cleanup.h, using cleanup helpers >> makes the code more readable overall compared to relying on multiple goto statements. >> Also, it’s not just about the "__free(...) = NULL" use cases -- there are locks >> involved as well. Switching to direct free() would require reverting those locks, >> since mixing cleanup helpers with manual cleanup is not acceptable. >> >> If this behavior is explicitly documented in cleanup.h, there is no reason not >> to use it as intended. I also support Krzysztof’s suggestion. > > It looks quite ugly, and it can't be mixed with the usual goto > cleanups (I suspect some care is needed with switch cases too), so we > must be careful where we use it. It's not obvious that this pattern > should be used in every function. However, where it's used, it should, > of course, be used correctly. > > Thanks for the fixes, Krzysztof. > > Amir, if you're happy with the fixes, can you give your R-B for each of them? > > Thanks, > Jens Thanks Jens. Sorry for the late response, I was on leave ;). I'll do. Regards, Amir