Re: [PATCH 1/9] xfsprogs: fix sign warnings in libxfs

[Christoph Hellwig <hch@infradead.org>]

On Thu, Jan 14, 2010 at 09:09:04PM +1100, Dave Chinner wrote:
> The directory naming is a mix of char and uchar_t and the compiler
> warns wheter they cross over. Cast the pointers according to the
> destination requirements as it doesn't seem to matter to avoid
> the warnings.

I think this makes sense, but it should really go into the kernel
code first - we currently paper over the warnings there by using
-funsigned-char for all XFS code which we might be able to remove
with those changes.

>  
>  struct xfs_name {
> -	const char	*name;
> +	const uchar_t	*name;
>  	int		len;

Maye use on of the standard types (unsigned char / u8 / uint8_t)
instead of the weird xfs-specific uchar_t, which in fact is already
gone in the kernel?

> @@ -1545,7 +1545,7 @@ xfs_attr_rmtval_get(xfs_da_args_t *args)
>  	ASSERT(!(args->flags & ATTR_KERNOVAL));
>  
>  	mp = args->dp->i_mount;
> -	dst = args->value;
> +	dst = (xfs_caddr_t)args->value;
>  	valuelen = args->valuelen;
>  	lblkno = args->rmtblkno;
>  	while (valuelen > 0) {
> @@ -1601,7 +1601,7 @@ xfs_attr_rmtval_set(xfs_da_args_t *args)
>  
>  	dp = args->dp;
>  	mp = dp->i_mount;
> -	src = args->value;
> +	src = (xfs_caddr_t)args->value;

If we make xfs_buf_iomove take a void * we can just pass on the
unsignedness in those two places.

> diff --git a/libxfs/xfs_attr_leaf.c b/libxfs/xfs_attr_leaf.c
> index f8f926f..d535752 100644
> --- a/libxfs/xfs_attr_leaf.c
> +++ b/libxfs/xfs_attr_leaf.c
> @@ -476,11 +476,11 @@ xfs_attr_shortform_to_leaf(xfs_da_args_t *args)
>  
>  	sfe = &sf->list[0];
>  	for (i = 0; i < sf->hdr.count; i++) {
> -		nargs.name = (char *)sfe->nameval;
> +		nargs.name = (uchar_t *)sfe->nameval;
>  		nargs.namelen = sfe->namelen;
> -		nargs.value = (char *)&sfe->nameval[nargs.namelen];
> +		nargs.value = (uchar_t *)&sfe->nameval[nargs.namelen];
>  		nargs.valuelen = sfe->valuelen;
> -		nargs.hashval = xfs_da_hashname((char *)sfe->nameval,
> +		nargs.hashval = xfs_da_hashname((uchar_t *)sfe->nameval,
>  						sfe->namelen);

Both the leaf and sf nameval fields already are __u8/__uint8_t aka
unsigned char, so I can't see why can't see why any casts are needed
here and in the later hunks dealing with namevals.

> @@ -489,7 +489,7 @@ xfs_dir2_block_lookup(
>  	 * Fill in inode number, CI name if appropriate, release the block.
>  	 */
>  	args->inumber = be64_to_cpu(dep->inumber);
> -	error = xfs_dir_cilookup_result(args, dep->name, dep->namelen);
> +	error = xfs_dir_cilookup_result(args, (char *)dep->name, dep->namelen);

xfs_dir_cilookup_result always gets unsigned char names now, so I'd move
the conversion into it.

>  	xfs_da_brelse(args->trans, bp);
>  	return XFS_ERROR(error);
>  }
> @@ -576,7 +576,7 @@ xfs_dir2_block_lookup_int(
>  		 * and buffer. If it's the first case-insensitive match, store
>  		 * the index and buffer and continue looking for an exact match.
>  		 */
> -		cmp = mp->m_dirnameops->compname(args, dep->name, dep->namelen);
> +		cmp = mp->m_dirnameops->compname(args, (char *)dep->name, dep->namelen);

Also here, ->compname appears to always get unsigned names.

> -		if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
> +		if (xfs_da_compname(args, (char *)sfep->name, sfep->namelen) ==
>  								XFS_CMP_EXACT) {
>  			ASSERT(xfs_dir2_sf_get_inumber(sfp,
>  						xfs_dir2_sf_inumberp(sfep)) ==
> @@ -928,8 +929,8 @@ xfs_dir2_sf_replace(
>  		for (i = 0, sfep = xfs_dir2_sf_firstentry(sfp);
>  				i < sfp->hdr.count;
>  				i++, sfep = xfs_dir2_sf_nextentry(sfp, sfep)) {
> -			if (xfs_da_compname(args, sfep->name, sfep->namelen) ==
> -								XFS_CMP_EXACT) {
> +			if (xfs_da_compname(args, (char *)sfep->name,
> +					sfep->namelen) == XFS_CMP_EXACT) {

Same seems to apply for direct calls to xfs_da_compname.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs