Function to check whether a path is valid or not needs your critique

Question

Here is my code, I use it to verify whether an input path is correct or not. I assume that the input path is encoded in UTF8. Is there any way to make it better?

#define MAX_PATH_LEN MAX_PATH

#define IN
#define OUT

typedef int Bool_T;

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

    int i;
    int n = strlen( path );

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

    return cnt;
}


Bool_T PathValid( IN const char * path )
{
    if( path != NULL )
    {
        if( GetCharsNumInPath( IN path ) <= MAX_PATH_LEN )
        {
            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 '|':
                        return false;

                    // Can meet only between a local disk letter and full path
                    // for example D:\folder\file.txt
                    //
                    case ':':
                    {
                        if( i != 1 )
                        {
                            return false;

                        }else{

                            break;
                        }
                    }

                    // Space and point can not be the last character of a file or folder names
                    //
                    case ' ':
                    case '.':
                    {
                        if( ( i + 1 == n ) || ( path[i+1] == PATH_SEPERATOR_CHAR ) )
                        {
                            return false;

                        }else{

                            break;
                        }
                    }

                    // two backslashes can not go straight
                    //
                    case PATH_SEPERATOR_CHAR:
                    {
                        if( i > 0 && path[i - 1] == PATH_SEPERATOR_CHAR )
                        {
                            return false;

                        }else{

                            break;
                        }
                    }
                }
            }

            return true;

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

            LOG_ERROR( "PathValid FAILURE --> path is too long" );
            return false;
        }

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

        // wrong argument
        //
        return false;
    }
}

Answer 1

Personally if the preconditions to the function are not met then I like to return quickly and this not add to the nesting:

Bool_T PathValid( IN const char * path )
{
    if( path != NULL )
    {
        // wrong argument
        return false;
    }

    if( GetCharsNumInPath( IN path ) <= MAX_PATH_LEN )
    {
        LOG_ERROR( "PathValid FAILURE --> path is too long" );
        return false;
    }

    // Main Function body here.
}

Of course in C you don't want to have too many return points from a function (as at each return point you need to close all open resources and tidy up. So in C (unlike C++) I find my code looking like this:

<type>   function(<Parameters>)
{
      if (!<pre-condition1>)
      {    return <errorStat1>;
      }

      if (!<pre-condition2>)
      {    return <errorStat2>;
      }

      ....

      if (!<pre-condition-N>)
      {    return <errorStat-N>;
      }


      <type>   result = <initial Result>;
      <Set all Resource to NULL>

      // Code Which can generate errors>
      // If it does then set error state. and stop processing.
      // BUT DO NOT RETURN from this code.

      < Close all resource >
      return result;
}

Your check for invalid characters is Windows specific. The OS will usually have a platform or OS specific set of functions for checking valid characters.

• Your set of invalid characters is not true for all platforms.
• The use of ':' as the second character is Windows only.
  • If it is there the first character should be 'A' - 'Z'
• Limiting the use of space is Windows only
• Are you sure about "." not being allowed as last character?
• On windows two '\\' can be used as the first two characters to indicate network resource.
• Windows has more than one path separator.
  • The old style '\' (traditional backslash)
  • The new style '/' (normal slash).

In your function counting characters in the path. You basically traverse the string twice. Why not combine the two traversals into a single pass:

// int n = strlen( path );
// Not requires (all this does is look for '\0')

for( i = 0; path[i]; i++ )
{       //  ^^^^^^^  Test for '\0'  character

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

Answer 2

Recognizing a path correctly is a bit more complex than you seem to realize (not surprising -- it's more complex than almost anybody seems to realize -- even quite a bit of Microsoft's software gets at least a few things wrong). I think I'd start by defining a grammar for what you're going to recognize. Since this seems to be based on Windows paths, I'd start with something like this:

path = local_path | share_path

local_path = <drive> FSpath

drive = [A-Za-z] ":"

share_path = "\\" share_name absolute_path

FSpath = relative_path | absolute_path

absolute_path = path_sep relative_path

relative_path = { name path_sep } file_name

path_sep = [\/]

share_name = name

name = [^?<>|\*:]+

file_name = name <fork_name>

fork_name = ":" name

Here I've used <whatever> to mean that item is optional and {whatever} to mean it can be repeated arbitrarily. [Edit: also be aware that I just typed this in based on what I remember -- I could easily have missed something so it's even a bit more complex than I've shown.]

Once you have that, it's pretty easy to write a small function for each of these to recognize that individual piece. To deal with optional parts of the path, I think I'd follow the convention of the function returning either the number of characters it matched from the input string, or else a pointer to the spot in the input string after the part it matched. Starting from the top, they'd look like:

bool path(char const *input) { 
    return local_path(input) || share_path(input);
}

size_t local_path(char const *input) { 
    size_t drive_len = drive(input);
    return drive_len + FSpath(input+drive_len);
}

size_t drive(char const *input) { 
    if (isalpha(input[0]) && input[1] == ':")
        return 2;
    return 0;
}

This style will probably result in more total code (especially overhead for function headers and such) but it breaks the problem down into pieces that are each small and simple so it should be fairly easy to figure out what's happening at each step of the way, so if (for example) my understanding of the overall syntax for a path is wrong, it should be fairly easy to find exactly which code relates to the part I have wrong. Likewise, when/if Microsoft changes the rules to add or remove restrictions, it should be quite easy to do that and be sure you're getting the right result. Just for example, right now I have share_name defined as being exactly the same as any other directory/file name. In all honesty, I'm not sure that's really correct. A share name may be more restricted than a normal directory name, but I'm honestly not sure -- but if it is different, this style makes it really easy to find and correct. This also makes it easy to deal with things that are only allowed at specific points along the path, or have different rules at different points (e.g., ':' being used for two different purposes: preceded by a drive letter, or followed by a named file fork on NTFS).