[bug report] fs: fat: create correct short names

[Heinrich Schuchardt]

On 26.07.23 09:02, Dan Carpenter wrote:
> Hello Heinrich Schuchardt,
>
> The patch 28cef9ca2e86: "fs: fat: create correct short names" from
> Nov 20, 2020, leads to the following Smatch static
> checker warning:
>
>      fs/fat/fat_write.c:61 str2fat()
>      warn: impossible condition '(c > 127) => ((-128)-127 > 127)'
>
>      fs/fat/fat_write.c:136 set_name()
>      warn: impossible condition '(*dirent.name == 229) => ((-128)-127 == 229)'
>
> fs/fat/fat_write.c
>      105 static int set_name(fat_itr *itr, const char *filename, char *shortname)
>      106 {
>      107         char *period;
>      108         char *pos;
>      109         int period_location;
>      110         char buf[13];
>      111         int i;
>      112         int ret;
>      113         struct nameext dirent;
>      114
>      115         if (!filename)
>      116                 return -EIO;
>      117
>      118         /* Initialize buffer */
>      119         memset(&dirent, ' ', sizeof(dirent));
>      120
>      121         /* Convert filename to upper case short name */
>      122         period = strrchr(filename, '.');
>      123         pos = (char *)filename;
>      124         if (*pos == '.') {
>      125                 pos = period + 1;
>      126                 period = 0;
>      127         }
>      128         if (period)
>      129                 str2fat(dirent.ext, period + 1, sizeof(dirent.ext));
>      130         period_location = str2fat(dirent.name, pos, sizeof(dirent.name));
>      131         if (period_location < 0)
>      132                 return period_location;
>      133         if (*dirent.name == ' ')
>      134                 *dirent.name = '_';
>      135         /* 0xe5 signals a deleted directory entry. Replace it by 0x05. */
> --> 136         if (*dirent.name == 0xe5)
>
> dirent.name is a char type.  Chars are signed on probably most people's
> systems.  The Linux kernel recently made char unsigned for everyone.
> Anyway in a char 0xe5 is negative but as a literal, it's positive so
> this condition cannot be true.
>
>      137                 *dirent.name = 0x05;

Thanks for reporting the issue. In the same file we have:

fs/fat/fat_write.c:1493:
         dent->nameext.name[0] = DELETED_FLAG;

include/fat.h:53:
#define DELETED_FLAG   ((char)0xe5)

I will send a patch.

Best regards

Heinrich

>      138
>      139         /* If filename and short name are the same, quit. */
>      140         sprintf(buf, "%.*s.%.3s", period_location, dirent.name, dirent.ext);
>      141         if (!strcmp(buf, filename)) {
>      142                 ret = 1;
>
> regards,
> dan carpenter