| Message ID | 20231107095156.365492-3-umang.jain@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | staging: vc04_services: Smatch fixes and remove custom logging | expand |
On Tue, Nov 07, 2023 at 04:51:49AM -0500, Umang Jain wrote: > In cases, where the global vchiq state is still unset, we cannot log > to dynamic debug (access to struct device is needed, hence potential > NULL de-reference). Log using pr_err() instead. No, something is wrong here, don't do that. > In vchiq_initialise(), remove the 'goto' because it is just again > trying to log to dynamic debug. Simply return with -ENNOTCONN after > logging to pr_err(). > > In vchiq_open(), move the vchiq_log_debug() after the state pointer > null check. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Reviewed-by: Ricardo B. Marliere <ricardo@marliere.net> > --- > .../staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 6 ++---- > .../staging/vc04_services/interface/vchiq_arm/vchiq_dev.c | 7 +++---- > 2 files changed, 5 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > index 9fb8f657cc78..9fb3e240d9de 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c > @@ -687,10 +687,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out) > usleep_range(500, 600); > } > if (i == VCHIQ_INIT_RETRIES) { > - vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not initialized\n", > - __func__); > - ret = -ENOTCONN; > - goto failed; > + pr_err("%s: videocore not initialized\n", __func__); > + return -ENOTCONN; Here's a good reason to get rid of the crazy "this subsystem is special so let us use a custom logging macro" logic. Convert everything to just use real dev_*() calls so it makes it obvious what is happening and how it all is working. It will save you from doing stuff like this in the future as you will "know" that there isn't a valid device pointer here. thanks, greg k-h
diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c index 9fb8f657cc78..9fb3e240d9de 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c @@ -687,10 +687,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out) usleep_range(500, 600); } if (i == VCHIQ_INIT_RETRIES) { - vchiq_log_error(state->dev, VCHIQ_CORE, "%s: videocore not initialized\n", - __func__); - ret = -ENOTCONN; - goto failed; + pr_err("%s: videocore not initialized\n", __func__); + return -ENOTCONN; } else if (i > 0) { vchiq_log_warning(state->dev, VCHIQ_CORE, "%s: videocore initialized after %d retries\n", __func__, i); diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c index 0bc93f48c14c..3425d2b199c2 100644 --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c @@ -1170,14 +1170,13 @@ static int vchiq_open(struct inode *inode, struct file *file) struct vchiq_state *state = vchiq_get_state(); struct vchiq_instance *instance; - vchiq_log_debug(state->dev, VCHIQ_ARM, "vchiq_open"); - if (!state) { - vchiq_log_error(state->dev, VCHIQ_ARM, - "vchiq has no connection to VideoCore"); + pr_err("%s: vchiq has no connection to VideoCore\n", __func__); return -ENOTCONN; } + vchiq_log_debug(state->dev, VCHIQ_ARM, "vchiq_open"); + instance = kzalloc(sizeof(*instance), GFP_KERNEL); if (!instance) return -ENOMEM;