Python-like C string library

Question

Most of the C code I have written has never been seen by anybody else, and I wonder if my code follows normal practices, especially how my code performs security-wise. The code is for a library that I have made for working with strings, and it is for use in other programs.

The full project can be found on GitHub.

The following is a snippet of the plstr.c file, the example usage comments were removed:

/**
 * @brief This function handels the cases for pl_translate where there is no
 * table. It should not be called directly, call pl_translate with the table
 * parameter set as NULL instead.
 */
char *translate_no_table(char *string, char *deletechars) {
    char *tmp = NULL, *ret_val = NULL;
    int i, x, found = 0, idx = 0;

    if (string == NULL || deletechars == NULL) {
        goto error_exit;
    }

    if (strlen(string) == 0 || strlen(deletechars) == 0) {
        goto error_exit;
    }

   for (i = 0; i < strlen(string); i++) {
        for (x = 0; x < strlen(deletechars); x++) {
            if (string[i] == deletechars[x]) {
                found++;
            }
        }
    }

    tmp = (char *) calloc(strlen(string) - found + 1, sizeof(char));
    if (tmp == NULL) {
        goto error_exit;
    }


    for (i = 0; i < strlen(string); i++) {
        int delete = 0;

        for (x = 0; x < strlen(deletechars); x++) {
            if (string[i] == deletechars[x]) {
                delete = 1;
            }
        }

        if (!delete) {
            tmp[idx] = string[i];
            idx++;
        }
    }

    ret_val = tmp;

    return ret_val;

error_exit:
    free(tmp);

    return ret_val;
}


/**
 * @brief This function handels the logic for the pl_translate function in the
 * cases where the table parameter is not empty. Do not call this function
 * directly, call pl_translate instead.
 */
char *translate_with_table(char *string, char *table, char *deletechars) {
    char *tmp = NULL, *ret_val = NULL;
    char swap_table[256];
    int i;

    if (string == NULL || table == NULL || deletechars == NULL) {
        goto error_exit;
    }

    if (strlen(string) == 0 || strlen(table) == 0 || strlen(deletechars) == 0) {
        goto error_exit;
    }

    if (strlen(table) != strlen(deletechars)) {
        goto error_exit;
    }

    for (i = 0; i < 256; i++) {
        swap_table[i] = i;
    }

    for (i = 0; i < strlen(table); i++) {
        swap_table[(int) table[i]] = deletechars[i];
    }

    tmp = pl_cpy(string, NULL);
    if (tmp == NULL) {
        goto error_exit;
    }

    for (i = 0; i < strlen(tmp); i++) {
        tmp[i] = swap_table[(int) tmp[i]];
    }

    ret_val = tmp;

    return ret_val;

error_exit:
    free(tmp);

    return ret_val;
}


/**
 * @brief This function implements the behaviour of python string translate
 * method. This function has two different behaviours. It either deletes
 * characters from the string, or switches out the characters with the ones
 * set in the table.
 *
 * If the table parameter, is \b NULL the characters passed in the deletechars
 * parameter are removed from the string. If you pass it the string
 * <a> 'read this short text'</a>, and deletechars is \a 'aeiou', the returned
 * string is <a>'rd ths shrt txt'</a>.
 *
 * If the table parameter is not empty, every occurrence of one of the table
 * characters is replaced with the character in deletechars at the same index.
 * So if the same string is passed <a>'read this short text'</a>, and the table
 * is \a 'xxxxx', and deletechars is \a 'aeiou' the returned string is
 * <a>'rxxd thxs shxrt txxt'</a>. The length of the table and deletechars needs
 * to be of the same length, or else the function will return \b NULL.
 *
 * You need to free the returned buffer after use.
 *
 * @param string The string you want to translate.
 *
 * @param table Optional parameter, if set it is used to swap the characters
 * passed in the deletechars parameter. If not every occurrence of the characters
 * passed in deletechars is removed from the string.
 *
 * @param deletechars The characters in the parameter is removed or swapped out
 * from the string. If the table parameter is used this parameter and table
 * needs to be of equal size.
 *
 * @return Returns a pointer to the new string with the deleted/swapped out
 * characters. If the function fails \b NULL is returned.
 *
 * \b Example
\code{.c}
\endcode
 *
 * \b Output
\code{.unparsed}
the_string: read this short text
no table:rd ths shrt txt
with table: rxxd thxs shxrt txxt
\endcode
 */
char *pl_translate(char *string, char *table, char *deletechars) {
    if (string == NULL || deletechars == NULL) {
        return NULL;
    }

    if (strlen(string) == 0 || strlen(deletechars) == 0) {
        return NULL;
    }

    if (table != NULL && strlen(table) == 0) {
        return NULL;
    }

    if (table == NULL) {
        return translate_no_table(string, deletechars);
    }

    return translate_with_table(string, table, deletechars);
}


/**
 * @brief This function splits up a string when a newline character is found.
 *
 * The function is very similar to the split function but the results are a bit
 * different. It splits up a string whenever it finds the \a \\n or \a \\r character.
 * If the keepends parameter is set the function will not remove the newline
 * from the string.
 *
 * If the string <a>"first\nsecond\n\nfourth"</a> is passed the results will be:
 * <a>["first", "second", "", "fourth"] </a>. If the keepends parameter is set
 * the results of the same string would be:
 * <a>["first\n", "second\n", "\n", "fourth\n"]</a>
 *
 * This function allocates memory, the returned pointer should be freed after
 * use.
 *
 * @param the_string The string you want to split.
 *
 * @param keepends If set to \a 1 the newlines will be appended, if not the
 * will be removed.
 *
 * @param size This parameter will be changed to the size of the returned
 * string array.
 *
 * @return The function will return a array of strings if successful, if the
 * function fails \b NULL will be returned.
 *
 * \b Example
\code{.c}
\endcode
 *
 * \b Output
\code{.unparsed}
0: first 1: second 2:  3: fourth

0: first

1: second

2:

3: fourth

\endcode
 */
char **pl_splitlines(char *the_string, int keepends, int *size) {
    char **ret_val = NULL, *pch = NULL, *tmp = NULL;
    int delims = 0, i, idx = 0;

    if (the_string == NULL || strlen(the_string) == 0)  {
        goto error_exit;
    }

    // Count the number of lines.
    for (i = 0; i < strlen(the_string); i++) {
        // 10 and 13 is the int value of \n and \r
        if ((int) the_string[i] == 10 || (int) the_string[i] == 13) {
            delims++;
        }
    }

    // Nothing todo.
    if (delims == 0) {
        goto error_exit;
    }

    ret_val = (char **) calloc(delims + 1, sizeof(char *));
    if (ret_val == NULL) {
        goto error_exit;
    }


    pch = the_string;
    for (i = 0; i < strlen(the_string); i++) {
        if ((int) the_string[i] == 10 || (int) the_string[i] == 13) {
            int len = (the_string + i) - pch;

            if (keepends) {
                len++;
            }

            tmp = (char *) calloc(len + 1, sizeof(char));
            if (tmp == NULL) {
                goto error_exit;
            }

            strncpy(tmp, pch, len);
            pch = the_string + i + 1;
            ret_val[idx] = tmp;
            idx++;
        }
    }

    tmp = (char *) calloc(strlen(pch) + 1, sizeof(char));
    if (tmp == NULL) {
        goto error_exit;
    }

    tmp = strncpy(tmp, pch, strlen(pch));
    ret_val[delims] = tmp;

    *size = delims + 1;

    return ret_val;

error_exit:
    if (delims) {
        for (i = 0; i < delims; i++) {
            free(ret_val[i]);
        }
    }

    free(ret_val);
    free(tmp);

    return ret_val;
}


/**
 * @brief Counts the number of occurences of a word in a string. The word can
 * does not need to be a word in the english langauge sense, just any form of
 * sub string can be counted.
 *
 * @param the_string The string you wnat to search.
 *
 * @param word The sub string you want to search the string for.
 *
 * @return Returns the number of occurences of the sub string. If the function
 * fails \b -1 is returned.
 *
 * \b Example
\code{.c}
\endcode
 * \b Output
\code{.unparsed}
The word 'three' was found 2 times.
Apparantly not 42. I always knew something was wrong with the universe.
\endcode
 */
int pl_count(char * the_string, char *word) {
    char *pch = the_string;
    int count = 0;

    if (the_string == NULL || word == NULL) {
        return -1;
    }

    if (strlen(the_string) == 0 || strlen(word) == 0) {
        return -1;
    }

    do {
        pch = strstr(pch, word);
        if (pch != NULL) {
            pch += strlen(word);
            count++;
        }
    } while(pch != NULL);

    return count;
}


/**
 * @brief This function replaces tabs with spaces.
 *
 * The tabs are replaced with the number of spaces in specified in the
 * \a tabsize parameter. There is no default value for the tabsize, so a value
 * needs to be passed. The resulting string is put in a heap allocated buffer,
 * the buffer should be freed after use.
 *
 * @param the_string The string with tabs.
 *
 * @param tabsize The number of spaces you want to replace each tab with.
 *
 * @return The function returns a heap allocated buffer of the string with tabs
 * replaced. If the function fails \b NULL is returned.
 *
 * \b Example
\code{.c}
\endcode
 *
 * \b Output
\code{.unparsed}
tabs    instead of      space
tabs instead of space
\endcode
 */

char *pl_expandtabs(char *the_string, int tabsize) {
    char *tmp = NULL, *ret_val = NULL;
    int i, tabcount = 0, str_size, idx;

    if (the_string == NULL || strlen(the_string) == 0 || tabsize < 0) {
        goto error_exit;
    }

    for (i = 0; i < strlen(the_string); i++) {
        // 9 is tab int value.
        if (the_string[i] == 9) {
            tabcount++;
        }
    }

    if (!tabcount) {
        goto error_exit;
    }

    str_size = (tabcount * tabsize) + strlen(the_string) + 1;
    tmp = (char *) calloc(str_size, sizeof(char));
    if (tmp == NULL) {
        goto error_exit;
    }

    idx = 0;
    for (i = 0; i < strlen(the_string); i++) {
        if (the_string[i] == 9) {
            int x;
            for (x = 0; x < tabsize; x++) {
                tmp[idx] = ' ';
                idx++;
            }
        }

        else {
            tmp[idx] = the_string[i];
            idx++;
        }
    }

    ret_val = tmp;

    return ret_val;

error_exit:
    free(tmp);

    return ret_val;
}

Answer 1

• Standard doesn't specify the signedness of char. The line

  swap_table[(int) table[i]] = deletechars[i];
  

  fails on a default-signed-char architecture (table[i] is promoted to negative value). Make it unsigned char * table.

• Functions which

  should not be called directly
  

  should not be exposed to the client. Make them static.

• translate_no_table

  • Counting found in is a waste of time. A resulting string is at most as large as the original.
  • translate_no_table should also use a lookup (like translate_with_table does ) table made from from deletechars. That would reduce complexity from NM to N+M (N, M being lengths of string and deletechars).

• Don't use magic numbers (9, 10, 13). They are '\t', '\r', '\n' really.

• pl_expandtabs has a semantical problem. It replaces the tab character with a given number of spaces. It is not what is usually expected. Instead, the number of spaces shall be calculated in such way that the character following the tab appears at a column which is multiple of tab width.

Answer 2

About strlen

You are calling strlen many many times on the same string(s). strlen(s) is iterating over s at every call (unless the compiler optimises it out for you).

Alternatives are :

• storing value in a variable : int l = strlen(string); for (i = 0; i < l; i++) { /* */ }.

• use a index and detect the end of the string on your own : for (i = 0; string[i] ]!='\0'; i++) { /* */ }.

• use a index and increment a pointer : for (const char* p = string; *p; ++p) { /* */}.

About variable declarations

It is a good habit to declare variable as late as possible, in the smallest possible scope. It makes things much easier to understand (and it can sometimes prevent you from using the wrong variable).

In you use C99, you can inline variable declaration as part of the for loop which is usually a good idea.

About goto

You'll read everything and its opposite about goto. As for most things, there is no definitive rule : sometimes it can be a good idea to use goto. In your case, you do not always need it.

Taking the two last comments into account, your first function becomes :

char *translate_no_table(char *string, char *deletechars) {
    if (string == NULL || deletechars == NULL ||
        strlen(string) == 0 || strlen(deletechars) == 0) {
        return NULL;
    }

    int found = 0;
    for (int i = 0; i < strlen(string); i++) {
        for (int x = 0; x < strlen(deletechars); x++) {
            if (string[i] == deletechars[x]) {
                found++;
            }
        }
    }

    char * tmp = (char *) calloc(strlen(string) - found + 1, sizeof(char));
    if (tmp == NULL) {
        return NULL;
    }

    int idx = 0;
    for (int i = 0; i < strlen(string); i++) {
        int delete = 0;

        for (int x = 0; x < strlen(deletechars); x++) {
            if (string[i] == deletechars[x]) {
                delete = 1;
            }
        }

        if (!delete) {
            tmp[idx] = string[i];
            idx++;
        }
    }

    return tmp;
}

It is shorter, easier to read and harder to get it wrong.