Date: Sun, 1 Feb 2015 16:00:34 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Dimitry Andric <dim@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r278004 - head/sys/dev/drm2/radeon Message-ID: <20150201153237.Q867@besplex.bde.org> In-Reply-To: <201501312218.t0VMIr7m035695@svn.freebsd.org> References: <201501312218.t0VMIr7m035695@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, 31 Jan 2015, Dimitry Andric wrote:
> Log:
> Constify a number of accesses in drm2's radeon drivers to avoid
> -Wcast-qual warnings. No functional change.
This is much better than using __DECONST(), but still has bogus casts.
>
> Modified: head/sys/dev/drm2/radeon/ni.c
> ==============================================================================
> --- head/sys/dev/drm2/radeon/ni.c Sat Jan 31 22:07:38 2015 (r278003)
> +++ head/sys/dev/drm2/radeon/ni.c Sat Jan 31 22:18:52 2015 (r278004)
> @@ -182,7 +182,7 @@ int ni_mc_load_microcode(struct radeon_d
> {
> const __be32 *fw_data;
> u32 mem_type, running, blackout = 0;
> - u32 *io_mc_regs;
> + const u32 *io_mc_regs;
> int i, ucode_size, regs_size;
>
> if (!rdev->mc_fw)
Part of the correct change.
> @@ -190,23 +190,23 @@ int ni_mc_load_microcode(struct radeon_d
>
> switch (rdev->family) {
> case CHIP_BARTS:
> - io_mc_regs = (u32 *)&barts_io_mc_regs;
Old bogus cast. The array has const elements, but the pointer was
not even to non-const elements (it was to an array type). A case was
used to break the warning about at least the non-const part of this.
But -Wcast-qual restored the warning.
> + io_mc_regs = (const u32 *)&barts_io_mc_regs;
New bogus cast. Now that the pointer type actually matches the data
element type, no cast is needed. However &barts_io_mc_regs isn't
actually a pointer to a data element. It is a pointer to the array,
which has a different type. The array type might actually be
important here, since the array is multi-dimensional and only a
pointer to an array could keep track of (most of) the dimensions.
However the code want a pointer to an element, since it wants to
treat the array as variable-size although its data is fixed-size,
and C's array indexing is no good except for fixed dimentsions.
> ucode_size = BTC_MC_UCODE_SIZE;
> regs_size = BTC_IO_MC_REGS_SIZE;
It keeps track of one of the dimensions here. The other one is presumably
fixed at 2. I forget whether it is the first or last dimension that is
not needed for array indexing.
> break;
So the bogus cast was also explicitly converting from an array type to
its element type. That conversion is normally done implicitly by
assignment because the types are compatible enough for this not to
break type safety. However, perhaps it is not actually safe for
multi-dimensional arrays or arrays with const elements. Really
strict type warnings would warn about implicitly and and explicitly
converting pointers to arrays. Anyway, it is clearer to point to the
first element:
io_mc_regs = &barts_io_mc_regs[0][0];
> case CHIP_TURKS:
> - io_mc_regs = (u32 *)&turks_io_mc_regs;
> + io_mc_regs = (const u32 *)&turks_io_mc_regs;
> ucode_size = BTC_MC_UCODE_SIZE;
> regs_size = BTC_IO_MC_REGS_SIZE;
> break;
Similarly for the other bogus casts.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20150201153237.Q867>