HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

A simple implementation of a mutable String in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationsimplemutablestring

Problem

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 
#include 
#include 
#include 

#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 blocks * BLOCK_SIZE) {
str->data[str->length] = c;
++(

Solution

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 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.

Code Snippets

while (str->data[pos] != c) ++pos;
while (pos < str->length && str->data[pos] != c) ++pos;
if (str != NULL) {
     // Rest of function indented
 }
if (str == NULL)
    return NULL;

// Rest of function, not indented

Context

StackExchange Code Review Q#156339, answer score: 2

Revisions (0)

No revisions yet.