Re: [PATCH v2] media: venus: add debugfs support

From: Al Viro
Date: Mon Jan 07 2019 - 11:55:29 EST


On Mon, Jan 07, 2019 at 07:59:00PM +0530, Malathi Gottam wrote:

> +static struct dentry *venus_debugfs_init_drv(void)
> +{
> + bool ok = false;
> + struct dentry *dir = NULL;
> +
> + dir = debugfs_create_dir("venus", NULL);
> + if (IS_ERR_OR_NULL(dir)) {
> + dir = NULL;
> + pr_err("failed to create debug dir");
> + goto failed_create_dir;
> + }

Huh? When does debugfs_create_dir() return ERR_PTR()?

Any interface that mixes returning NULL for error-reporting with returning
ERR_PTR(...) for the same needs to be put out of its misery. As it happens,
debugfs_create_dir() returns NULL on all errors. And if you've meant the
above as "future-proofing"... don't do that, please. That only breeds
confusion down the road, as the code gets cut'n'pasted around.

> - if (ret)
> + if (ret) {
> + dprintk(ERR,
> + "Failed to set actual buffer count %d for buffer type %d\n",
> + buf_count.count_actual, buf_count.type);
> return ret;
>
> buf_count.type = HFI_BUFFER_OUTPUT;
> buf_count.count_actual = output_bufs;
>

*blink*
Does that even compile?