On 25/06/16 22:36, Greg KH wrote: > On Thu, Jun 23, 2016 at 01:36:17PM +0100, Luis de Bethencourt wrote: >> The common format to check if a function returned an error pointer is to >> use PTR_ERR(). Instead of ERR_PTR() which is used to return said errors. >> >> Signed-off-by: Luis de Bethencourt <luisbg@xxxxxxxxxxxxxxx> >> Reviewed-by: Julian Calaby <julian.calaby@xxxxxxxxx> >> --- >> drivers/staging/wilc1000/wilc_debugfs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/staging/wilc1000/wilc_debugfs.c b/drivers/staging/wilc1000/wilc_debugfs.c >> index fcbc95d..48797dc 100644 >> --- a/drivers/staging/wilc1000/wilc_debugfs.c >> +++ b/drivers/staging/wilc1000/wilc_debugfs.c >> @@ -107,7 +107,7 @@ static int __init wilc_debugfs_init(void) >> struct wilc_debugfs_info_t *info; >> >> wilc_dir = debugfs_create_dir("wilc_wifi", NULL); >> - if (wilc_dir == ERR_PTR(-ENODEV)) { >> + if (PTR_ERR(wilc_dir) == -ENODEV) { >> /* it's not error. the debugfs is just not being enabled. */ >> printk("ERR, kernel has built without debugfs support\n"); >> return 0; > > No, the best way to do this is to just ignore the return value, you > don't care about it. It can be passed back into any debugfs calls just > fine. > > So don't check the value and all is good, debugfs was written in a way > to make it _easy_ to use, no need for fancy error checking at all with > it. > > thanks, > > greg k-h > Thanks for the review Greg. Just to make sure. You are proposing I just drop the 3 if checks? [0] If that's what you mean I will send a patch as soon as you confirm :) Happy hacking, Luis [0] Making the function look like this: static int __init wilc_debugfs_init(void) { int i; struct dentry *debugfs_files; struct wilc_debugfs_info_t *info; wilc_dir = debugfs_create_dir("wilc_wifi", NULL); for (i = 0; i < ARRAY_SIZE(debugfs_info); i++) { info = &debugfs_info[i]; debugfs_files = debugfs_create_file(info->name, info->perm, wilc_dir, &info->data, &info->fops); } return 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html