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:   Tue, 10 Aug 2021 18:14:27 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Evgeny Novikov <novikov@ispras.ru>
Cc:     Mauro Carvalho Chehab <mchehab@kernel.org>,
        Sakari Ailus <sakari.ailus@linux.intel.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Alex Dewar <alex.dewar90@gmail.com>,
        Colin Ian King <colin.king@canonical.com>,
        Linus Walleij <linus.walleij@linaro.org>,
        linux-media@vger.kernel.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, ldv-project@linuxtesting.org
Subject: Re: [PATCH] media: atomisp: Fix error handling in probe
Message-ID: <20210810151427.GY1931@kadam>
References: <20210810115303.9089-1-novikov@ispras.ru>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210810115303.9089-1-novikov@ispras.ru>
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 JN2P275CA0035.ZAFP275.PROD.OUTLOOK.COM (2603:1086:0:2::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4415.13 via Frontend Transport; Tue, 10 Aug 2021 15:14:44 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 56279926-6678-4277-9d34-08d95c119a28
X-MS-TrafficTypeDiagnostic: MWHPR10MB1358:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <MWHPR10MB13581890761A12058C94D42E8EF79@MWHPR10MB1358.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:8273;
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: SHFqG4zCxppY2A/Aw8XHs2URfD3dWAq+c5we6vWQes3mVt5ybm6S+yljuwsEHOa6SsfT+bPdlE0yFXPHr3p1IRLhFePc+sSyt/YZVNXR6uCxUbNy08Dk2PW46lowIOPTMFglO8JeWJP4xwu/rs1j2gxmkML1Ou8Lm8f8cj8hDc+eKUVMytUUhaZAtlAONAzaYpHCSIqxDmMp67zLk4jnfZ0gPeNMzF0GQXAA8soG5viNj7wpYOvAjr0CVBX6Ab4e3IkhLYuYejKwLVJISeT4lRJwp1YRxtli+Bmyslyq4EYZ6UXGPl9SKC8ayjwjzzmvTii9o0a4uGpCNcjv7K//4FeNbCczYW61NOr68TXJuacuZdNvXC+19R8h/G4AEyAGObB50qAduxNoFa/FqvCwvzo5A8daDIbvTmcX83nW0QPRKHG3847c6sVsEgfT+C/ijYjItnTZ5kmTaNFSR7cw6xd280kW05bH8iQdqC79cyLLOSE45HIIAb2rEmVK7BklAMRXBLRBRZs4+zZGcd0TWyATWaThYcboSSmIOdnYYy3149GKNYDSJi3totArXz5e6UeoacpJb3pkfHnB1X2ootAdJIBVdnToun8H+YdhsH+fuy2GQKnOyS+APPi6iVIDujTDiWDRBc75jR6dBaGtin8iJBvrMbVCdDWJMu/wWMIjYSOAsuFWui6roTiP1jA4OyojJVY4/iP6sVbIEaF3wQ==
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:(396003)(39860400002)(366004)(376002)(136003)(346002)(9576002)(54906003)(52116002)(6496006)(186003)(9686003)(33716001)(38350700002)(38100700002)(66556008)(55016002)(66476007)(86362001)(478600001)(83380400001)(66946007)(4326008)(7416002)(6916009)(5660300002)(316002)(2906002)(44832011)(956004)(1076003)(6666004)(26005)(33656002)(8936002)(8676002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?seUawm7Z9IxDy8HhEnwyngixb3zK2dytM5/MK7p6G1YCVjrxOJPSpBJ3eVtU?=
 =?us-ascii?Q?42To4Xxcw+qCCUSADYIvHCNFyr4YENzeLw1RSh5HudrpOqIofJmlGc+zZ+dY?=
 =?us-ascii?Q?GNQ+QkaJsr2PXIJPonEWz1+49xJsXOtZkapC8I1iBnrGVT3cvq42QoOR2Lnd?=
 =?us-ascii?Q?Op4fU/0ZcWKBc4+zsy/H6yl5mavJJ6EVG8WgN19GRGjbtWROf6QiPWg/cMjU?=
 =?us-ascii?Q?pWqpiQXIXtgeSDh9bdwsialFjU/V7uYsPyoJ6pgc5yuBHfQEFmemhi1C85ye?=
 =?us-ascii?Q?WW++qeP0QnDX9DKR/IZOU0HPRgZOuQ+SGtGNod8wVpDS/TyhW0MNZdSBriv9?=
 =?us-ascii?Q?8Jy5eJWJPRSqhPH6Ci3azDHmGDl+axEzwp++MIycy1Mhb5I3Pd9tmCs6MhjU?=
 =?us-ascii?Q?xDWHexlm5F6dwDGIog90lTUqA2IlPU7SJ3ak2SwLjPIeXywG+NON+I5rHZHQ?=
 =?us-ascii?Q?bfj/rKuSTH3uV9xxvnXoacJHlwDEKm/aBQsE9dUdZCYguwgm27sPXP2iTyf/?=
 =?us-ascii?Q?YdljVdi5eTZutaQIqXeBnqWzM24QBzmCyJC9R0JBHkw/vewSBiYCsu6VNj6P?=
 =?us-ascii?Q?SwyrSfVCxMWuz5l1LvCGqDhk3IWrSz96u+KzUq0rWEqwEfT6y5sJnLs7uYQW?=
 =?us-ascii?Q?K3x7LjxUtu2AGnFd9DmWJcXtWKM4jsA+RaWoOjYER/gFnsRyxDKObi6G2LZD?=
 =?us-ascii?Q?YdzqBShyhS8bCejn3EQKTS2/gaELSYCQ7RrdC9q5Sp3DOTcnFWaG0yyFpZxk?=
 =?us-ascii?Q?c4yYI08nrAyoul+TbtMmZ5mzcuuS8ibpY5AmDdV/LVaJjt6+rY5GOVoxOpPR?=
 =?us-ascii?Q?vQ3VeMsvX9HBUf4JC2b7XQ9PvFzg8y/B3ItAAfKtDpZoKAowdscwR/kXSrfJ?=
 =?us-ascii?Q?FIh+T4BT6sxgM5AaYgSzAb8whghCTQcGWIpXWlN85SgpfiXLdi9DOpBA0Stk?=
 =?us-ascii?Q?Mt11ATrxj/D/tRHIh6jMmvY8AT/7aCSu+gyUjzB8B9Vl9efswAs9qpbmSGdx?=
 =?us-ascii?Q?WzeXWIp85u586dT2f/bGuRPdDhjQsDZDA/O7K4n/kNP+HvuUZi9Y6MA90L1f?=
 =?us-ascii?Q?AZT0Ms3AiGbLuPvpY8W99jMB+wT9ksn3T984LjtA2SFHIaCvnFYqeILSXvR4?=
 =?us-ascii?Q?uocLNlxOCOxa9vu27RHjRmjTJkepqB30JPunpu7Ml+KCYbXoCM22QPhAXBx9?=
 =?us-ascii?Q?BuqnQrH+jKQg3Sx/Z/huO7gjEfl+4cybsbZ4DgeT4Xo7erAnzOOWVtCqRCYW?=
 =?us-ascii?Q?TOdcyoeSCil3uykOwqdpd/mEVI2aO8PvkpVX1FG/eVtSQ+CMR6NF3TRUwstN?=
 =?us-ascii?Q?ET2VvXwflWMnDfpQ5ki6tUVK?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 56279926-6678-4277-9d34-08d95c119a28
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Aug 2021 15:14:52.7404
 (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: q+I8mdnUO6E/Z3bpqJZri93xzTofxVrIVjPPkgV1B7Tj/ivP36958gUg4U3N9RBvdZJ+QcpwZd7I+PwirhUTaTUuMcNqsO+GHDrpGc4rQgo=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1358
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=10072 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0 mlxscore=0
 phishscore=0 spamscore=0 mlxlogscore=705 malwarescore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2107140000
 definitions=main-2108100097
X-Proofpoint-ORIG-GUID: 7kcEP6PKFehOJ33gVXjjno_5dpuuNoVd
X-Proofpoint-GUID: 7kcEP6PKFehOJ33gVXjjno_5dpuuNoVd
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Aug 10, 2021 at 02:53:03PM +0300, Evgeny Novikov wrote:
> There were several issues with handling errors in lm3554_probe():
> - Probe did not set the error code when v4l2_ctrl_handler_init() failed.
> - It intermixed gotos for handling errors of v4l2_ctrl_handler_init()
>   and media_entity_pads_init().
> - Probe did not free resources in case of failures of
>   atomisp_register_i2c_module().
> 
> The patch fixes all these issues.
> 
> Found by Linux Driver Verification project (linuxtesting.org).

This patch is good, but two things:

1) Please can you rename all the labels to say what the goto does?

err_uninit:
	lm3554_gpio_uninit(client);
cleanup_media:
 	media_entity_cleanup(&flash->sd.entity);
free_handler:
 	v4l2_ctrl_handler_free(&flash->ctrl_handler);
unregister_subdev:
 	v4l2_device_unregister_subdev(&flash->sd);

2) There is a missing error code still:

drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
   858          if (ret) {
   859                  dev_err(&client->dev, "error initialize a ctrl_handler.\n");
   860                  goto fail3;
   861          }
   862  
   863          for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++)
   864                  v4l2_ctrl_new_custom(&flash->ctrl_handler, &lm3554_controls[i],
   865                                       NULL);
   866  
   867          if (flash->ctrl_handler.error) {
   868                  dev_err(&client->dev, "ctrl_handler error.\n");
   869                  goto fail3;
                        ^^^^^^^^^^
	err = flash->ctrl_handler.error;

   870          }
   871  
   872          flash->sd.ctrl_handler = &flash->ctrl_handler;
   873          err = media_entity_pads_init(&flash->sd.entity, 0, NULL);
   874          if (err) {
   875                  dev_err(&client->dev, "error initialize a media entity.\n");
   876                  goto fail2;
   877          }

regards,
dan carpenter