0
\$\begingroup\$

It is a function that validates a string is a correct windows path to a directory. It returns negative numbers if it isn't, 0 if it is correct and 1 if it also exists:

int path_is_correct_directory (char *path)
{
    size_t  szPath          = 0U;

    if(path == NULL)
        return -1;
    if(*path == '\0')
        return -2;

    szPath = strlen(path);

    if(szPath < 3 || szPath > MAX_PATH)
        return -3;

    char    driveLetter[3]      = { path[0], path[1], '\0' };
    BOOL    validDriveLetter    = PathIsDirectory(driveLetter);

    if(validDriveLetter == 0)
        return -4;

    if(path[2] != '\\')
        return -5;

    int i;
    for(i = 3; i < szPath; i++)
    {
        if(path[i] == '/')  return -6;
        if(path[i] == ':')  return -7;
        if(path[i] == '*')  return -8;
        if(path[i] == '"')  return -9;
        if(path[i] == '<')  return -10;
        if(path[i] == '>')  return -11;
        if(path[i] == '|')  return -12;
        if(path[i] == '?')  return -13;
    }

    if(PathIsDirectory(path))
        return 1;

    return 0;
}
\$\endgroup\$
  • \$\begingroup\$ Don't you have access to Dir? \$\endgroup\$ – Raystafarian Mar 16 '18 at 4:19
  • 1
    \$\begingroup\$ @Raystafarian what do you mean? \$\endgroup\$ – Edenia Mar 16 '18 at 11:31
2
\$\begingroup\$

Magic numbers

All error codes are magic numbers. It's hard to keep numbers in mind, though. If I use

path_is_correct_directory(some_path)

and get -6, what exactly did I do wrong? I have to look into the documentation, check the error values and add that to my own code:

if(path_is_correct_directory(some_path) == -6) {
    yell_at_user("Path may not contain slash!");
}

But that's still opaque. Instead, use an enum:

enum RESULTS {
    PATH_IS_VALID     = 0,
    PATH_IS_DIRECTORY = 1,
    /*   ---   */
    PATH_IS_NULL   = -1,
    PATH_IS_EMPTY  = -2,
    PATH_TOO_SHORT = -3,
    /*   ---   */
    INVALID_DRIVE           = -4,
    INVALID_DRIVE_DELIMITER = -5,
    /*   ---   */
    PATH_CONTAINS_SLASH         = -6,
    PATH_CONTAINS_COLON         = -7,
    PATH_CONTAINS_ASTERISK      = -8,
    PATH_CONTAINS_QUOTE         = -9,
    PATH_CONTAINS_LT            = -10,
    PATH_CONTAINS_GT            = -11,
    PATH_CONTAINS_PIPE          = -12,
    PATH_CONTAINS_QUESTION_MARK = -13
};

Instead of magic numbers, I can now confidently compare against a nice name:

if(path_is_correc_directory(some_path) == PATH_IS_DIRECTORY) {
    list_directory_contents(some_path);
}

Keep scope of variables short

Since you use C99, you can keep the scope of your variables shorter. For example i should only be valid in the for loop, but not outside. szPath can get initialized with strlen right away (after the NULL check).

Variables that shouldn't change can be const

All szPath, path driveLetter and validDriveLetter should not change and can therefore get changed to const.

sz is a prefix for strings, not numbers

In Hungarian notation sz is a prefix for "string terminated by zero". Since Hungarian notation is used throughout Windows APIs, that can be a source of confusion. path_size is fine on the other hand.

All at once

int path_is_correct_directory (const char *path)
{
    if(path == NULL)
        return PATH_IS_NULL;
    if(*path == '\0')
        return PATH_IS_EMPTY;

    const size_t path_size = strlen(path);

    if(path_size < 3 || path_size> MAX_PATH)
        return PATH_TOO_SHORT;

    const char    driveLetter[3]      = { path[0], path[1], '\0' };
    const BOOL    validDriveLetter    = PathIsDirectory(driveLetter);

    if(validDriveLetter == 0)
        return INVALID_DRIVE;

    if(path[2] != '\\')
        return INVALID_DRIVE_DELIMITER;

    for(int i = 3; i < path_size; i++)
    {
        if(path[i] == '/')  return PATH_CONTAINS_SLASH;
        if(path[i] == ':')  return PATH_CONTAINS_COLON;
        if(path[i] == '*')  return PATH_CONTAINS_ASTERISK;
        if(path[i] == '"')  return PATH_CONTAINS_QUOTE;
        if(path[i] == '<')  return PATH_CONTAINS_LT;
        if(path[i] == '>')  return PATH_CONTAINS_GT;
        if(path[i] == '|')  return PATH_CONTAINS_PIPE;
        if(path[i] == '?')  return PATH_CONTAINS_QUESTION_MARK;
    }

    if(PathIsDirectory(path))
        return PATH_IS_DIRECTORY;

    return PATH_IS_VALID;
}

By the way, Windows also supports network directories, which start with \\.

\$\endgroup\$
  • \$\begingroup\$ To handle network directory he can rely on PathIsUNCA from the same header than PathIsDirectory. \$\endgroup\$ – Calak Oct 27 '18 at 16:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.