Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 22 Mar 2022 11:55:58 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc:     David Kershner <david.kershner@unisys.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        sparmaintainer@unisys.com, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: unisys: Properly test debugfs_create_dir()
 return values
Message-ID: <20220322085558.GN3293@kadam>
References: <20220322083858.16887-1-fmdefrancesco@gmail.com>
 <20220322084928.GM3293@kadam>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220322084928.GM3293@kadam>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?/LKksA21NheImts1f1YDlAJytfTKagb686p6MWFe4rHyHpcaKaUrUbDqapPV?=
 =?us-ascii?Q?MzZNhaRSt9unnybJr9KMzlLcIr9IHLdOc4M7sLqTufqssR1PnjJyp+xsiyZP?=
 =?us-ascii?Q?2EPT01I8trg1VqYR1GdLXSTSGtxGbqftHi6HbI0EkQ4u0PLUC5ErswxPQV4r?=
 =?us-ascii?Q?2od+cAlCq3Ze3AOZg9+607gGlFsMs3hRFw/K/0cfmlf8tlXjYfIFwD2IKaVW?=
 =?us-ascii?Q?6KnU5Bah7qoxSUdxEgEZQ7d/b9V95aimhd+1MRr6UIzx7ax60vGLysMbaK7J?=
 =?us-ascii?Q?ylfQNRKy+GkEmtX7U1AzfhGruplQE5sqkcq61L+X6t2WiPrvoaFwqLfDtARa?=
 =?us-ascii?Q?eaWiEDEL8CnqkSDxdFchOjTTICEY9Tmzsy2HNHHQgBgUnZQqv4hPhbshAAM6?=
 =?us-ascii?Q?9H+DUtRNPnFp9dsFmWAGR7f0Lcyzd0qYPs9b89uQs+NgVOTEc0BqFdFrUdSU?=
 =?us-ascii?Q?a2tOA6ZLymP9dT/XMXFN+6+JLZfEBoqSNV7lqXvsTLpEKnBs6Cu3fC4bnmK/?=
 =?us-ascii?Q?icoASRCtJBYdWca00Gp9dZHLxutxfnDHViKbihz6uzk3RCLt7VMo1ehMtQq6?=
 =?us-ascii?Q?uJNNH3w0fN/hdYjMXNobjvvIRAOl2g3JuDWv5aVnSU29GocvQBEVYgVDkzSH?=
 =?us-ascii?Q?UD8+k6eyFamevKVWw2YbNnjatxsnMjdeShlNOqZI1o6Ib466e3fEWL2iBsCj?=
 =?us-ascii?Q?SI6S6XpxRlSQNh1e8riYVBIsfFYHFGItfVbkPqqnAyBfc7Lqa8UCmpg+biZE?=
 =?us-ascii?Q?ZBeIHF6I2KCSEff3CF+z3S7XBfy1E74iDvfQHclXaHP74qzFHqLf7vIEpNH8?=
 =?us-ascii?Q?vQZ6f8Ko+n1TdYkAnGhJxdPtk2B0CkSBub3KfalNTQzbksTXriA6SWEvGZFb?=
 =?us-ascii?Q?bRgnsHDpLTPQzpjhMlHmk6oZIz7aX9d87m9peVD+vxpMDxCyfOTwYItRiBKp?=
 =?us-ascii?Q?4BJWr8t0Uge6iYPeDxLiRG3mpFuyfZpylEOTtxanVyjcCADolPoE9dE2+BHK?=
 =?us-ascii?Q?WzPyxLS3AA2Gj2EG7WBo/ljaf6sc7cmKGP+oXmrreIiXPc+sfqR0/TMJqrQy?=
 =?us-ascii?Q?hjLqbN6e5bZGuAsHvEVZqziy0oXGKE/9UBeFFT1iE9l6siHTG/x80GjY+o4w?=
 =?us-ascii?Q?bxtw60B6quF5lzS0XgO+rBFnvr3r+Lr0NwYS2YsaMliUZB88V+eaHVPhd1mQ?=
 =?us-ascii?Q?SnoznrcLHGjbPsdcucwpqSdBvgE9X36gWl5BPemii96BsYkG5/U4jrmXvnR+?=
 =?us-ascii?Q?wprSRS/hP0hzhtaGla75qyYNr1IaVBOubTf6RhiP2MPxVy+44iiArJ/xpw4c?=
 =?us-ascii?Q?nT74A2Tbt34BYz4QCihTTkRcmCp1LG/Hu2O6dxXfGvokVLsUa3jQlN4OtkW8?=
 =?us-ascii?Q?vnVAFGDJMLfDHRgb8o1UE+HEt48O8BGvtDzWZU1pK4cohspxNASTyZ1hYvOU?=
 =?us-ascii?Q?wysxl51nmTxRwbpC8zuKLlDE9aoHIC7XMmzB+qNJCmHE24jhZFWnzg=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 646f2449-fa4b-4b3e-7206-08da0be1d002
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Mar 2022 08:56:11.4839
 (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: 9xut2z2ErB3GWK3GUNzGAwMwgpm10AYXateat45N42493P6ycIPf9im6quUG83BP6Bjrhggsen1nnDdoWcq5hoHKRGGX9JzkOldKuVPhxoY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5433
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10293 signatures=694221
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 malwarescore=0 mlxscore=0
 bulkscore=0 phishscore=0 spamscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2203220053
X-Proofpoint-GUID: CsbYC8dGMIpmTll02qc13Zd-gpQ0QDFW
X-Proofpoint-ORIG-GUID: CsbYC8dGMIpmTll02qc13Zd-gpQ0QDFW
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Mar 22, 2022 at 11:49:28AM +0300, Dan Carpenter wrote:
> On Tue, Mar 22, 2022 at 09:38:58AM +0100, Fabio M. De Francesco wrote:
> > debugfs_create_dir() returns a pointers to a dentry objects. On failures
> > it returns errors. Currently the values returned to visornic_probe()
> > seem to be tested for being equal to NULL in case of failures.
> > 
> > Properly test with "if (IS_ERR())" and then assign the correct error 
> > value to the "err" variable.
> > 
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> >  drivers/staging/unisys/visornic/visornic_main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
> > index 643432458105..58d03f3d3173 100644
> > --- a/drivers/staging/unisys/visornic/visornic_main.c
> > +++ b/drivers/staging/unisys/visornic/visornic_main.c
> > @@ -1922,11 +1922,11 @@ static int visornic_probe(struct visor_device *dev)
> >  	/* create debug/sysfs directories */
> >  	devdata->eth_debugfs_dir = debugfs_create_dir(netdev->name,
> >  						      visornic_debugfs_dir);
> > -	if (!devdata->eth_debugfs_dir) {
> > +	if (IS_ERR(devdata->eth_debugfs_dir)) {
> 
> Normally I would say to just delete the error handling.

In the normal case checking debugfs functions for errors is not
required.  The users all have checks built in.  The exception is when
the driver dereferences devdata->eth_debugfs_dir itself.  So when you
make these kinds of changes do a grep for eth_debugfs_dir to make sure
it's not dereferenced, but normally it is not.

But again, here, delete everything.

regards,
dan carpenter