Functions in C language that manipulate strings, not present in the standard C library

Question

I have implemented few functions in C language that manipulate strings. These functions are not present in the standard C library.

---

string_library.c

#include <stdio.h>
#include <stdlib.h>

#include "string_library.h"


/*
 * get_input_from_stdin_and_discard_extra_characters(char *str, long size):
 *
 * Function get_input_from_stdin_and_discard_extra_characters() reads at most
 * 'size - 1' characters into 'str' from stdin and then appends the NULL
 * character ('\0'). In all cases, reading input stops after encountering a
 * newline ('\n') or EOF even if 'size - 1' characters have not been read.
 * If a newline ('\n') or EOF is read then these are replaced by NULL character
 * ('\0'). If there are extra characters in input, they are read and discarded.
 * In all cases, str is returned.
 *
 */
char *get_input_from_stdin_and_discard_extra_characters(char *str, long size)
{

    char c = 0;
    long num_chars_to_read = size - 1;
    long i = 0;

    if (!str)
        return NULL;

    if (num_chars_to_read <= 0)
        return NULL;

    for (i = 0; i < num_chars_to_read; i++) {

        c = getchar();

        if ((c == '\n') || (c == EOF)) {
            str[i] = 0;
            return str;
        }

        str[i] = c;

    } // end of for loop

    str[i] = 0;

    // discard rest of input
    while ((c = getchar()) && (c != '\n') && (c != EOF));

    return str;

} // end of get_input_from_stdin_and_discard_extra_characters


int is_string_empty(const char *str)
{

    if (!str)
        return 1;

    if (*str == 0)
        return 1;

    return 0;

} // end of is_string_empty


int is_string_all_whitespace(const char *str)
{

    char *temp = (char *)(str);

    if (is_string_empty(str))
        return 1;

    while (*temp) {
        if ((*temp != ' ') && (*temp != '\t'))
            return 0;
        temp++;
    }

    return 1;

} // is_string_all_whitespace


/*
 * This function fills 'str' with 'len - 1' random characters and appends a null
 * byte at the end and returns 'str'.
 */
char *get_random_string(char *str, long len)
{

    int min_num = 32;
    int max_num = 126;
    int i = 0;

    for (i = 0; i < (len - 1); i++) {
        str[i] = (random() % (max_num - min_num + 1)) + min_num;
    }

    str[len - 1] = 0;

    return str;

}


/*
 * str_replace_chr(char *str, const char orig, const char new):
 *
 * Function str_replace_chr() replaces all occurrences of 'orig' character in
 * the string 'str' with the 'new' character. If 'str' is NULL then nothing is
 * done. In all cases, 'str' is returned.
 *
 */
char *str_replace_chr(char *str, const char orig, const char new)
{

    char *orig_str = (str);

    if (!str)
        return orig_str;

    while (*str) {
        if (*str == orig)
            *str = new;
        str++;
    }

    return orig_str;

} // end of str_replace_chr


/*
 * substr(const char *str, long start_index, long end_index):
 *
 * Function substr() allocates memory and returns a pointer to a string / character
 * array which is a substring of 'str' starting from index 'start_index' till
 * 'end_index' (inclusive). This substring is terminated by NULL byte at the end.
 * If 'str' is NULL or 'start_index' is less than 0 or 'end_index' is less than 0
 * or 'end_index' is less than 'start_index' then NULL is returned.
 *
 * The returned pointer points to a memory region containing the substring and this
 * memory region was allocated using malloc. So, it is the user's responsibility to
 * free the allocated memory.
 *
 */
char *substr(const char *str, long start_index, long end_index)
{

    char *substring = NULL;
    long length = 0;
    long i = 0;

    if (!str)
        return NULL;

    if ((start_index < 0) || (end_index < 0) || (end_index < start_index))
        return NULL;

    length = end_index - start_index + 1;

    substring = malloc(length + 1); // extra 1 byte for NULL byte
    if (!substring)
        return NULL;

    for (i = 0; i < length; i++) {
        substring[i] = str[i + start_index];
    }

    substring[i] = 0;

    return substring;

} // end of substr

Answer 1

get_input_from_stdin_and_discard_extra_characters

    c = getchar();

c is a char, so we lose the ability to distinguish EOF here. We need to use int for that.

The whole function could be made much simpler: just use fgets(), then if the final character read was \n, overwrite it, else discard the rest of the line with scanf("%*[^\n]"),scanf("%*c").

is_string_empty

The name is very close to those reserved for standard library extension, but is legal because is is followed by underscore rather than a lower-case letter.

It would be better named as is_string_null_or_empty, as a null pointer is quite different from an empty string. It could be simplified, too:

#include <stdbool.h>

bool is_string_empty(const char *str)
{
    return !str || !*str;
}

is_string_all_whitespace

Don't cast away const like that. temp can be a pointer to const char.

I'm surprised that only space and tab are considered - why not everything that's isspace()?

This too could be much simpler:

#include <ctype.h>

bool is_string_null_or_all_whitespace(const char *s)
{
    if (!s) { return true; }
    for (;;) {
        if (!*s) { return true; }
        if (!isspace((unsigned char)(*s++))) { return false; }
    }
}

get_random_string

Is there any value in returning the pointer that was passed? Consider a return type of void instead.

The length should probably be a size_t rather than long.

Those min and max values suggest that you're assuming the target is using ASCII - if so, that assumption should be documented (and preferably tested so that where it's untrue, users get a compilation error rather than random behaviour).

Is there a good reason not to use the standard library rand() for this function?

#include <assert.h>

void get_random_string(char *str, size_t len)
{
    assert(' ' == 32);          /* This function assumes ASCII */
    static const char min_num = 32;
    static const char max_num = 126;

    while (len--) {
        *str++ = (char)(rand() % (max_num - min_num + 1) + min_num);
    }
    *str = '\0';
}

substr

If we passed the indices as size_t, we wouldn't need to check for negative values.

In modern C, we don't need to declare all our variables at beginning of scope. It's better to wait until we can initialise them.

We should use the standard memcpy() function to copy the contents. Good compilers will recognise the loop and replace with memcpy(), but it's easier to use the function than to write the loop correctly.

We'll get undefined behaviour if we attempt to copy off the end of the array. We could use strncpy() to finish when we reach a null character, or we could measure the length and check that at the start of the function.

I like the good handling for when malloc() returns a null pointer.

#include <string.h>

char *substr(const char *str, size_t start_index, size_t end_index)
{
    if (!str || !*str ) {
        return NULL;
    }
    const char *end = memchr(str, '\0', end_index);
    if (end) {
        end_index = end - str - 1;
    }
    if (end_index < start_index) {
        return NULL;
    }

    const size_t length = end_index - start_index + 1;

    char *const substring = malloc(length + 1); // include terminating null char
    if (substring) {
        memcpy(substring, str + start_index, length);
        substring[length] = '\0';
    }

    return substring;
}

Overall

I'm disappointed to see no unit tests of these functions. Including tests in the review helps us understand what's known to work, and often tells us about the anticipated use cases. I encourage you to include tests whenever possible!

Answer 2

substr

This function does not work as I expected it to. If I called it like in the following:

substr("abc", 1, 1);

I would have expected it to return an empty string "" and not "b".

You should try to limit your function to do a single task. substr allocates a new string and writes to it. You can leave the allocation up to the caller.

Suggested replacement:

int substr(char* dst, size_t size, const char* src, size_t start, size_t length)
{
    if (length >= size || start + length > strlen(src))
        return -1;
    memmove(dst, src + start, length);
    dst[length] = 0;
    return length;
}

This has two advantages over your implementation:

1. You do not have to remember to free the results
2. It can be used on pre-existing buffers.

Consider the following

struct ipaddress
{
    char part1[4];
    char part2[4];
    char part3[4];
    char part4[4];
};

int main()
{
    char* test = "196.168.332.556";
    struct ipaddress adr;
    substr(adr.part1, 4, test, 0, 3);
    substr(adr.part2, 4, test, 4, 3);
    substr(adr.part3, 4, test, 8, 3);
    substr(adr.part4, 4, test, 12, 3);
}

If you used your implementation, you would have had to do an extra strcpy and remembered to free the string.

You will also be able to use the same buffer for the source and destination like so:

int main()
{
    char test[] = "123ABCDEF456"; 
    substr(test, sizeof(test), test, 3, 6);
    printf("%s\n", test);
}

str_replace_chr

Here you could have used strchr to search for the occurrences of orig:

void str_replace_chr(char* str, const char orig, const char new)
{
    while (str = strchr(str, orig))
    {
        *str++ = new;
    }
}

Answer 3

Functional difference from fgets()

I'd expect get_input_from_stdin_and_discard_extra_characters() to perform like fgets() when there is no input. Instead get_input_from_stdin_and_discard_extra_characters(str, ...) returns str. fgets(str, ...) returns NULL.

Types

Size of buffer and array indexing is more idiomatic C as size_t than long. Even int has some precedent, but not long.

Use int c to save result from fgetc() to well distinguish the 257 different responses. Avoid infinite loops when char is unsigned and fgetc() returns EOF.

Pedantic: signed math overflow

Perform if (num_chars_to_read <= 0) before long num_chars_to_read = size - 1; to avoid signed math overflow.

Missing string_library.h

Documentation

External functions deserve documentation in the .h file. Consider the user may not have access to the .c file (nor want it). Documentation in the .c file explains implementation. Overall functionality, higher level, is for the .h.

Name space

string_library.c contains various functions that have no or little name connection to string_library.*. Consider a more uniform naming convention.

get_input_from_stdin_and_discard_extra_characters() is too long.

Pedantic: range

get_random_string(str,len) is UB when len <= 0.

WET ()

() not needed in char *orig_str = (str);.

NULL is for pointers

Rather then comment

// extra 1 byte for NULL byte
// use 
extra 1 byte for null character
// or
extra 1 byte for \0
// or
extra 1 byte for nul

NULL is the null pointer constant. It may be a pointer type.

null character is what the C spec uses.

'\0' is another good choice

nul is defined outside C in ASCII.

Well uniformly formatted

No noticed spelling/grammar issues