From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BFC5CC6FD20 for ; Sat, 25 Mar 2023 19:24:14 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 079746B0072; Sat, 25 Mar 2023 15:24:14 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 029626B0074; Sat, 25 Mar 2023 15:24:13 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E33806B0075; Sat, 25 Mar 2023 15:24:13 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id D1F776B0072 for ; Sat, 25 Mar 2023 15:24:13 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id A2BAC1A01A4 for ; Sat, 25 Mar 2023 19:24:13 +0000 (UTC) X-FDA: 80608396386.03.57DB72C Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) by imf11.hostedemail.com (Postfix) with ESMTP id C25B940004 for ; Sat, 25 Mar 2023 19:24:11 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Pocj9XuB; spf=pass (imf11.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.51 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1679772251; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tcnVA78wkVzbPfXfOJ2LEcC0Y+HKxsbDQcgFdv0NWhs=; b=jDpxFY3t4CHC8o5E+7ERN5dXQPLDp6+mInXQ/0SGdNfxioUVhSHscKZGPS8HcRPgRZjcnb HeWkruFSoilAEY/ZBvm63x1vXkaFTXbAA8hez7sW4QMmz4oIi2dI0a0lvOIVQ84W24LfEl k7F5FaetIOqtz0/AqkJpIg2h3/Knge0= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=Pocj9XuB; spf=pass (imf11.hostedemail.com: domain of lstoakes@gmail.com designates 209.85.128.51 as permitted sender) smtp.mailfrom=lstoakes@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1679772251; a=rsa-sha256; cv=none; b=dn//SToHifVevK3SmxP0n3ySDmasq4wg3koYcXy/WdAhC8sPDIZpC5D8ed8Xe+oqxZpD2Q j+tNhTIMwLd/GtLByglfnyJWkKV3URoM/v+8eVoRF78NfUFLxx02w5EEOnGvqC+slP9tyL HkjEqUD3vRST2PPLgpO2nJSxpEg2HwQ= Received: by mail-wm1-f51.google.com with SMTP id fm20-20020a05600c0c1400b003ead37e6588so4994706wmb.5 for ; Sat, 25 Mar 2023 12:24:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; t=1679772250; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=tcnVA78wkVzbPfXfOJ2LEcC0Y+HKxsbDQcgFdv0NWhs=; b=Pocj9XuByR3+O92iq2mpvWIyQvZt/GKsOhuWboG4VjE1ElKkX0M4H1lkjCsl8CpzN7 S+ad7YC4Ka3kz9ZCYnsWJks1NnAiyglsLk+s6xRkdqAIUP00quC16G8Z9x34sQQnbVbh jXBzC2ksIhiCRSVP/MOIxnJmAhqk8Q6+f/7JQGHIzgfUrQe7Yv8LsVLO1lYNvzya9RKz U28nihqEM2vbM0F4Kt/8TRoNk0jFm74V7HhKDqzntjVTk83BhJ3v+NIHGkJYLjZJWVAS mLB6Ocrk+OH0cPdE/HE/QAXFeE7sILEp4687gd04apIH9gcHkTvWB1FuHftWvYlbmvPv vTSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1679772250; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tcnVA78wkVzbPfXfOJ2LEcC0Y+HKxsbDQcgFdv0NWhs=; b=Sbhok4ClAJkGCY4N3rL6MZQqNI+5l7Xa7luyWH6Jy+GXBBS82Vy3JhYMw4BMiFK15f yY1wQgtCkI3jOyGDAEA6X7oT2lA1SI5IYClMA63OucW88DHsOlRg2Fc1iyM96yMNBlyj esgy9TM6OworCZpf9Qfj5Lz6GQm12I48KjNjSlDvpqkUCUQ1HG/fATFU1vG15/4EccC7 DDq4OskKQVoFMg3iLotT/em5C/5EKSgcIKLnNvPPu/su0lyfl9pKILjViZJZSFA6eCOb x7Q518UwKoYUJkGGir2umdwp6IrMRgrNV3gsJksmUL7hGDdSiJvi4jVScYC3Glm0O16Z R4Dw== X-Gm-Message-State: AO0yUKXZvNjjFbr1d0ibF6U6GU6qFqkgI8pctUDvYXi0+XKOaVYVMvb5 6cnc4PEFeDr9gMQrnB9ulHI= X-Google-Smtp-Source: AK7set/U2CC6eWEHlcDRZJcCoHMyfxS7P7nSp0cEP3JJ0eMW68/jHSmYtAF9YnCen6KiCRPQ65XWwA== X-Received: by 2002:a05:600c:286:b0:3ee:7f0b:387b with SMTP id 6-20020a05600c028600b003ee7f0b387bmr4567366wmk.17.1679772250121; Sat, 25 Mar 2023 12:24:10 -0700 (PDT) Received: from localhost ([2a00:23c5:dc8c:8701:1663:9a35:5a7b:1d76]) by smtp.gmail.com with ESMTPSA id t9-20020a1c7709000000b003ed1ff06faasm8573061wmi.19.2023.03.25.12.24.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 25 Mar 2023 12:24:09 -0700 (PDT) Date: Sat, 25 Mar 2023 19:24:08 +0000 From: Lorenzo Stoakes To: Markus Elfring Cc: kernel-janitors@vger.kernel.org, linux-kselftest@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org, Jay Kamat , Johannes Weiner , Michal Hocko , Muchun Song , Roman Gushchin , Shakeel Butt , Shuah Khan , Tejun Heo , Zefan Li , cocci@inria.fr, LKML Subject: Re: [PATCH] selftests: cgroup: Fix exception handling in test_memcg_oom_group_score_events() Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam03 X-Stat-Signature: mfa6ns3p4t1yjbyot94zw8cgyqyq3x6f X-Rspamd-Queue-Id: C25B940004 X-HE-Tag: 1679772251-593154 X-HE-Meta: U2FsdGVkX1/RQOkRzeOvuMvrWZqT+VAOe0oI7uY8KAj+eyx6xiMRu5n0XTc+vJdYw4Es7dxSt5BpRaeGT4OOXboqXTLr/vk3f151loRb8MArR8KM6NgzwBP7qREQxhT7CgixCpDTmN+fSEPO/WAmC5yLfkpIx/mv4th08CKsfRvJLgd0M4nWQCvaDS0al2AwV8z+9XXw6EDZtiowcIpaztlnBMjUqEiHcjOtFrJgbsnBj1vqOQSm2B4t0I6niYabNwOJVHGB9OL6lGxVemk/k0WtaDcLNyuTEeO0YAq8GpHiBY92+cQoPoE2qFHH5W3frjZ6vabLLfx9WkUkh5JKhmf4hVJOFYSr2IlsTicYEebJ6dCy0lDFsZGfwl2IgBuexa4j9VsGsK16TGIlZIawnzkIcTrttCNvylphL+MtpPz7TZH9CquKJcHnft2wsjTDJhe+pmFW9cIsJyjvxtrMIrCIQ6aWbe6J59Nw2fGDus28KD9RuWWfR5gTbB3c8t1m9e/FgUfUreQJ0ws5KEX+pRqDWfBYJ+nkrcRHQsBVEirL+V/09UDtdcbCWCY0H7YFEhBEsgpYg24Pry08ROwWO7Cu5qpHIlQT4PypaWCm53ReP6SJSFqmjJ72Yn9gQcPUpsuGe7waffiAyA4P9HN1deTf8Te8bWsNGZArFdkphTKwwrKBg1B2m0PnIQmQK4/Dqu279+2N5n6h6rNDmZsE44cuOs+aSiPUg2uaoxSqceMrqp57w+2548cEosJQv88d6k6EwL1psjgb/wxe09s0Y0C78+l+s4yV2NZiHf0gYRN8N+DSTdxbcGvF3KlIccSGiR5jXLRxMCtcS0YXfVFf7JgwJH6cCmRAHvm6QFjVtDDSlrLqrWYv3m2NvU5zi0ggsSWU9pXGDRPOVed+SbNOn+tyCKVzPurrI6nBUcYQmkfeIustDpYtHzKujwHTvM9jeC8zFXDKmuiciQClCuo vWK1v0rf 1a211j6wTXY1haQHM23nr+3HnIjralqmMbX1TdkSYC3wxtgA/YmAOieSRAZLXgusgCQjgFtjBKBVNrGurRju5GifhAdiPisHTJFKTEZeN6B/C8DMTMfVcmuvF8ri1znFW4npEubUdoage8FwlY1jmex2/iVnfu3Xh/WOEuxB6a0Tfbo4cOD2wM9v8S13O1A7fvQEg+Qr7xxXDjkRP2i7janXdlXJL/rN35s6jnh52nRNZtvEgWodROHgGffQZXEt3NmkfP0GARoT9j99fN8pmzbd8TF3Ur+DUoclWSVrDJ6L14pzCntCz4PIvwr/Y49ZJtdrfQxUY/wZaArIN6dVzLSOjzmsvj36Gcp3SjcnOWUcKyV3FeYg1oZn6od7nUxR16L0Eobpk3lmv71AbzG8tlL2Vf0nx7RQ5tkXDpTtBo91Te9nApRcyDrhdWCcfLbneVR8BkXY38KwkRwBTW5CfHMt7xby62q7W6UhUQooaHudcr1s= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Sat, Mar 25, 2023 at 07:30:21PM +0100, Markus Elfring wrote: > Date: Sat, 25 Mar 2023 19:11:13 +0100 > > The label “cleanup” was used to jump to another pointer check despite of > the detail in the implementation of the function > “test_memcg_oom_group_score_events” that it was determined already > that a corresponding variable contained a null pointer. This is poorly writte and confusing. Something like 'avoid unnecessary null check/cg_destroy() invocation' would be far clearer. > > 1. Thus return directly after a call of the function “cg_name” failed. > This feels superfluious. > 2. Use an additional label. This also feels superfluious. > > 3. Delete a questionable check. This seems superfluois and frankly, rude. It's not questionable, it's readable, you should try to avoid language like 'questionable' when the purpose of the check is obvious. > > > This issue was detected by using the Coccinelle software. > > Fixes: a987785dcd6c8ae2915460582aebd6481c81eb67 ("Add tests for memory.oom.group") Fixes what in the what now? This is not a bug fix, it's a 'questionable' refactoring. > Signed-off-by: Markus Elfring > --- > tools/testing/selftests/cgroup/test_memcontrol.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git a/tools/testing/selftests/cgroup/test_memcontrol.c b/tools/testing/selftests/cgroup/test_memcontrol.c > index f4f7c0aef702..afcd1752413e 100644 > --- a/tools/testing/selftests/cgroup/test_memcontrol.c > +++ b/tools/testing/selftests/cgroup/test_memcontrol.c > @@ -1242,12 +1242,11 @@ static int test_memcg_oom_group_score_events(const char *root) > int safe_pid; > > memcg = cg_name(root, "memcg_test_0"); > - > if (!memcg) > - goto cleanup; > + return ret; > > if (cg_create(memcg)) > - goto cleanup; > + goto free_cg; > > if (cg_write(memcg, "memory.max", "50M")) > goto cleanup; > @@ -1275,8 +1274,8 @@ static int test_memcg_oom_group_score_events(const char *root) > ret = KSFT_PASS; > > cleanup: > - if (memcg) > - cg_destroy(memcg); > + cg_destroy(memcg); > +free_cg: > free(memcg); > > return ret; > -- > 2.40.0 > > I dislike this patch, it adds complexity for no discernible purpose and actively makes the code _less_ readable and in a self-test of all places (!) Not all pedantic Coccinelle results are actionable. Remember that it's humans who are reading the code. Your email client/scripting is still somehow broken, I couldn't get b4 to pull it correctly and it seems to have a duplicate message ID. You really need to take a look at that (git send-email should do this fine for example). Please try to filter the output of Coccinelle and instead of spamming thousands of pointless patches that add no value, try to choose those that do add value. My advice overall would be to just stop.