3
\$\begingroup\$

The following code implements a simple interface to operate on mutable* Strings in C. It is composed of two files: one for the structure definition and the available operations, and the other with the actual implementation.

*This can be inexact, as only adding characters to the end is implemented.

Any comments on coding style and improvements are greatly appreciated.

String.h

#ifndef __STRING_H__
#define __STRING_H__

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

#define BLOCK_SIZE 256

typedef struct {
    char *data;
    int length;
    int blocks;
} String;

String * string_create();
void string_dispose(String *str);

bool string_empty(String *str);
void string_append_char(String *str, char c);

char * string_get_all(String *str);
char string_get(String *str, int pos);

int string_find(String *str, char c);

#endif /* __STRING_H__ */

String.c

#include "String.h"

/** Create a String */
String * string_create() {
    String *ans = calloc(1, sizeof *ans);

    if (ans != NULL) {
        ans->data = malloc( BLOCK_SIZE * sizeof *(ans->data) );

        ans->length = 0;
        ans->blocks = (ans->data == NULL) ? 0 : 1;
    }

    return ans;
}

/** Free the memory associated with a String */
void string_dispose(String *str) {
    if (str != NULL) {
        free(str->data);
        free(str);
    }
}

/** Is the String empty? */
bool string_empty(String *str) {
    if (str == NULL) return true;
    if (str->length == 0) return true;
    return false;
}

/** Add a character to the end of the String */
void string_append_char(String *str, char c) {
    if (str != NULL) {
        if (str->length == str->blocks * BLOCK_SIZE) {
            char *new_str = realloc( str->data, BLOCK_SIZE * (str->blocks + 1) * sizeof *(str->data));

            if (new_str != NULL) {
                str->data = new_str;
                ++(str->blocks);
            }
        }

        if (str->length < str->blocks * BLOCK_SIZE) {
            str->data[str->length] = c;
            ++(str->length);
        }
    }
}

/** Get a C-String with the proper null-terminator */
char * string_get_all(String *str) {
    char *res = NULL;

    if (str != NULL) {
        res = malloc((str->length + 1) * sizeof *str->data);

        if (res != NULL) {
            memcpy(res, str->data, str->length);
            res[str->length] = '\0';
        }
    }

    return res;
}

/** Get a character at a given position in the String */
char string_get(String *str, int pos) {
    char res = '\0';

    if (str != NULL) {
        if (pos >= 0 && pos < str->length) {
            res = str->data[pos];
        }
    }

    return res;
}

/** Get where the first occurrence of a character in the String is */
int string_find(String *str, char c) {
    int pos = -1;

    if (str != NULL) {
        pos = 0;
        while (str->data[pos] != c) ++pos;

        if (pos == str->length) pos = -1;
    }

    return pos;
}

This is a test file that uses some of the core methods implemented in a String. This file is called test_string.c.

#include <stdio.h>

#include "String.h"

int main() {
    String *str = string_create();

    char i;
    for (i = 'a'; i <= 'z'; i++)
        string_append_char(str, i);

    char *cstr = string_get_all(str);

    printf("%s\n", cstr);

    int pos = string_find(str, 'f');

    printf("Character 'f' occurs at position %d.\n", pos);
    printf("Reading from the String, we get \"%c\".\n", string_get(str, pos));

    free(cstr);
    string_dispose(str);

    return 0;
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Unsafe loop

This loop in string_find() is unsafe since it could read past the end of your buffer:

    while (str->data[pos] != c) ++pos;

You should add an additional check like this:

    while (pos < str->length && str->data[pos] != c) ++pos;

Short circuit error conditions

Rather than doing this:

 if (str != NULL) {
     // Rest of function indented
 }

it would be easier to read if you rewrote it like this:

if (str == NULL)
    return NULL;

// Rest of function, not indented

Other things

Your reallocation strategy will lead to \$O(n^2)\$ behavior when appending to long strings. You might want to double the allocation instead of adding a fixed amount.

You might want to use size_t for your sizes and lengths instead if int, because an int might overflow at 32KB on some platforms.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your feedback! I'll take into account all your suggestions. I actually found the first issue while testing - I am now using strchr for finding, but didn't know why it was failing. I'll post a revised version soon. \$\endgroup\$ Commented Feb 26, 2017 at 6:43

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.