Re: git: 4710aa248bcd - main - Avoid dereferencing a possibly null pointer.

From: Tijl Coosemans <tijl_at_FreeBSD.org>
Date: Sat, 09 Apr 2022 19:54:12 UTC
On Wed, 6 Apr 2022 21:26:14 GMT Kirk McKusick <mckusick@FreeBSD.org>
wrote:
> The branch main has been updated by mckusick:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=4710aa248bcdd77811540ad5695270254edfff55
> 
> commit 4710aa248bcdd77811540ad5695270254edfff55
> Author:     Kirk McKusick <mckusick@FreeBSD.org>
> AuthorDate: 2022-04-06 21:25:11 +0000
> Commit:     Kirk McKusick <mckusick@FreeBSD.org>
> CommitDate: 2022-04-06 21:25:55 +0000
> 
>     Avoid dereferencing a possibly null pointer.
>     
>     Reported by: Coverity
>     CID:         1475868
> ---
>  sys/geom/union/g_union.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/sys/geom/union/g_union.c b/sys/geom/union/g_union.c
> index 5a4977a5fa57..23f7eaac25a9 100644
> --- a/sys/geom/union/g_union.c
> +++ b/sys/geom/union/g_union.c
> @@ -1317,31 +1317,31 @@ g_union_destroy(struct gctl_req *req, struct g_geom *gp, bool force)
>  		if (force) {
>  			if (req != NULL)
>  				gctl_msg(req, "Device %s is still in use, "
> -				    "so is being forcibly removed.", pp->name);
> +				    "so is being forcibly removed.", gp->name);
>  			G_UNION_DEBUG(1, "Device %s is still in use, so "
> -			    "is being forcibly removed.", pp->name);
> +			    "is being forcibly removed.", gp->name);
>  		} else {
>  			if (req != NULL)
>  				gctl_msg(req, "Device %s is still open "
> -				    "(r=%d w=%d e=%d).", pp->name, pp->acr,
> +				    "(r=%d w=%d e=%d).", gp->name, pp->acr,
>  				    pp->acw, pp->ace);

Shouldn't the 3 remaining pp be replaced as well?

>  			G_UNION_DEBUG(1, "Device %s is still open "
> -			    "(r=%d w=%d e=%d).", pp->name, pp->acr,
> +			    "(r=%d w=%d e=%d).", gp->name, pp->acr,
>  			    pp->acw, pp->ace);

And here too?

>  			return (EBUSY);
>  		}
>  	} else {
>  		if (req != NULL)
> -			gctl_msg(req, "Device %s removed.", pp->name);
> -		G_UNION_DEBUG(1, "Device %s removed.", pp->name);
> +			gctl_msg(req, "Device %s removed.", gp->name);
> +		G_UNION_DEBUG(1, "Device %s removed.", gp->name);
>  	}
>  	/* Close consumers */
>  	if ((error = g_access(sc->sc_lowercp, -1, 0, -1)) != 0)
>  		G_UNION_DEBUG(2, "Error %d: device %s could not reset access "
> -		    "to %s.", error, pp->name, sc->sc_lowercp->provider->name);
> +		    "to %s.", error, gp->name, sc->sc_lowercp->provider->name);
>  	if ((error = g_access(sc->sc_uppercp, -1, -1, -1)) != 0)
>  		G_UNION_DEBUG(2, "Error %d: device %s could not reset access "
> -		    "to %s.", error, pp->name, sc->sc_uppercp->provider->name);
> +		    "to %s.", error, gp->name, sc->sc_uppercp->provider->name);
>  
>  	g_wither_geom(gp, ENXIO);
>