Function to validate a file name

Question

This function checks if a file name is valid or not. I think it's not good enough for integration (not optimized, maybe I forgot about some checks), so need your comments.

static
int ReservedName( const char * name )
{
    char * reservedFileNames[] = { "CON",  "PRN",  "AUX",  "NUL",
                                   "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
                                   "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9" };

    char * reservedFileNamesWithDot[] = { "CON.",  "PRN.",  "AUX.",  "NUL.",
                                          "COM1.", "COM2.", "COM3.", "COM4.", "COM5.", "COM6.", "COM7.", "COM8.", "COM9.",
                                          "LPT1.", "LPT2.", "LPT3.", "LPT4.", "LPT5.", "LPT6.", "LPT7.", "LPT8.", "LPT9." };

    int i;
    int n = sizeof( reservedFileNames ) / sizeof( reservedFileNames[0] );

    for( i = 0; i < n; i++ )
    {
        if( stricmp( reservedFileNames[i], name ) == 0 ) 
        {
            return 1;

        }else{

            if( strnicmp( reservedFileNamesWithDot[i], name, strlen( reservedFileNamesWithDot[i] ) ) == 0 )
            {
                return 1;
            }
        }
    }

    return 0;
}


static
int GetCharsNumInPath( IN const char * path )
{
    int i, cnt;

    for( i = 0; path[i]; i++ )
    {
        if( ( path[i] < 0x80 ) || ( path[i] > 0xbf ) )
            cnt++;
    }

    return cnt;
}


int ValidName( const char * name )
{
    if( name != NULL )
    {
        if( GetCharsNumInPath( path ) <= MAX_PATH )
        {
            if( ! ReservedName( name ) )
            {
                int i;
                int n = strlen( path );

                for( i = 0; i < n; i++ )
                {
                    switch( path[i] )
                    {
                        // ? " / < > * |
                        // these characters can not be used in file or folder names
                        //
                        case '<':
                        case '>':
                        case '/':
                        case '\\':
                        case '\"':
                        case '?':   
                        case '*':
                        case '|':
                            return 0;

                        case ' ':
                        case '.':
                        {
                            if( ( i + 1 == n ) )
                            {
                                return 0;

                            }else{

                                continue;
                            }
                        }
                    }
                }

                return 1;

            }else{ // if( ! ReservedName( name ) )

                return 0;
            }

        }else{ // if( GetCharsNumInPath( IN path ) <= MAX_PATH_LEN )

            return 0;
        }

    }else{ // if( name != NULL )

        return 0;
    }
}

Answer 1

Every one has their own side of the holy war about where the '{' should be.

But one thing everybody agrees upon is that you must be consistent.
In your code you align the open and close (which I think is perfect) apart from when you use else

   }else{

Stay with the same style as the rest of your code

   }
   else
   {

If your 2 arrays are supposed to be identical then you should probably not have two different arrays. You should probably use a single array and use code to get the extra functionality. Otherwise you head into a maintenance problem where every addition to one array must be mirrored by an addition to the other array (and you must validate they are the same).

char * reservedFileNames[] = { "CON",  "PRN",  "AUX",  "NUL",
                               "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", "COM9",
                               "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9" };

   size_t  wordLen = strlen( reservedFileNames[i]);
   if( stricmp( reservedFileNames[i], name, wordLen) == 0 ) 
   {
       if (name[wordLen] == '\0')   // The strings are technically equal
       {    return 1;
       }
       if (name[wordLen] == '.')    // The string has a bad prefix.
       {    return 1;
       }
   }

Answer 2

I'm not sure what you're using this for, but my advice: Don't bother validating the filename. Just open the file. The system knows the rules better than you do, and by adding too much validation you can cause problems down the line.

On Windows these rules are especially complex. Frankly you will never find them all. Here's a few you missed, off the top of my head:

• Filenames may contain unicode characters. You are using char*. Unless you're using UTF-8 and passing all your filenames to MultiByteToWideChar before they reach a file API, you're ruling out a lot of potential valid paths based on that. (Not to mention the way these filenames get mapped to Unicode is now subject to the user's language settings, which one can argue is a bad thing in its own right...)
• MAX_PATH. You're assuming that all paths will fit in a buffer of that size. On Windows you can actually have a path longer than MAX_PATH if it starts with \\?\; also things like \\?\C:\foo are valid paths.
• Names containing trailing spaces or dots, which you seem to be filtering on, are also valid and distinct if the path begins with \\?. Without that prefix, trailing dots and spaces get truncated.
• Paths under \\.\, \\?\GLOBALROOT.

Not to mention, these rules could potentially be subject to change. Note that Win9X had different rules than NT. What if such a change occurs again and breaks your logic? Don't bother duplicating these rules in your code; don't second-guess the filesystem either. Just pass the name to CreateFile.

(Aside: If your filename happens to come from an untrusted source, then it probably is fair to be overly restrictive...)