Re: [PATCH] xfs_db: fix complaints about unsigned char casting

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Mar 17, 2023 at 06:43:40PM +0800, Alan Huang wrote:
> Is there any reason keep these definition with different types?  Question from a newbie...

Probably not, but char* handling in C is a mess if you don't specify
signedness -- on some arches gcc interprets "char" as "signed char", and
on others it uses "unsigned char".  The C library string functions all
operate on "char *", so if you start changing types you quickly end up
in compiler warning hell over that.

libxfs seems to use "unsigned char*" and "uint8_t *".  I don't know if
that was just an SGI thing, or merely a quirk of the codebase?  The
Linux VFS code all take "char *", though as of 6.2 the makefiles force
those to unsigned everywhere.  As far as directory entry and extended
attribute names go, all eight bits are allowed.

UTF8 encoding uses the upper bit of a char, which means that we really
want unsigned to avoid problems with sign extension when computing
hashes and things like that, because emoji and kanji characters are in
use around the world.

IOWS: it's a giant auditing headache to research what parts use the type
declarations they do, figure out what subtleties go with them, and
decide on appropriate fixes.  And in the end... the system still behaves
the same way it did.

--D

> Thanks,
> Alan
> 
> > 2023年3月17日 下午6:25，Carlos Maiolino <cem@kernel.org> 写道：
> > 
> > On Tue, Mar 14, 2023 at 06:01:10PM -0700, Darrick J. Wong wrote:
> >> From: Darrick J. Wong <djwong@kernel.org>
> >> 
> >> Make the warnings about signed/unsigned char pointer casting go away.
> >> For printing dirent names it doesn't matter at all.
> >> 
> >> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > 
> > Looks good, will test.
> > 
> > Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
> > 
> >> ---
> >> db/namei.c |    4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/db/namei.c b/db/namei.c
> >> index 00e8c8dc6d5..063721ca98f 100644
> >> --- a/db/namei.c
> >> +++ b/db/namei.c
> >> @@ -98,7 +98,7 @@ path_navigate(
> >> 
> >> 	for (i = 0; i < dirpath->depth; i++) {
> >> 		struct xfs_name	xname = {
> >> -			.name	= dirpath->path[i],
> >> +			.name	= (unsigned char *)dirpath->path[i],
> >> 			.len	= strlen(dirpath->path[i]),
> >> 		};
> >> 
> >> @@ -250,7 +250,7 @@ dir_emit(
> >> 	uint8_t			dtype)
> >> {
> >> 	char			*display_name;
> >> -	struct xfs_name		xname = { .name = name };
> >> +	struct xfs_name		xname = { .name = (unsigned char *)name };
> >> 	const char		*dstr = get_dstr(mp, dtype);
> >> 	xfs_dahash_t		hash;
> >> 	bool			good;
> > 
> > -- 
> > Carlos Maiolino
>