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

A dynamic string interface in C

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

Problem

So I've written up what the title says.

Obviously, I'm looking for speed, safety and ease of use. To achieve the perfect ratio I've made some functions straight away crash if the input is wrong (in debug mode only, otherwise UD). The design is inspired by the C++ counterpart.

Will be glad to hear other opinions. (It's a lot of code, so bear with me.)

dynstring.h

#ifndef DYNSTRING_H
#define DYNSTRING_H

#include 
#include 

typedef struct {
    char* data;
    size_t size;
} string;

extern bool string_create(string*);
extern bool string_create_n(string*, size_t);
extern bool string_create_from_raw(string*, const char*);

extern void string_destroy(string*);

extern bool string_copy(string*, string);
extern bool string_copy_from_raw(string*, const char*);

extern void string_set(string*, size_t, char);
extern char string_get(string, size_t);

extern char* string_front(string);
extern char* string_back(string);

extern char* string_to_raw(string);

extern size_t string_size(string);
extern bool   string_empty(string);

extern void string_erase(string*, size_t, size_t);

extern bool string_push_back(string*, char);
extern bool string_push_front(string*, char);

extern bool string_pop_back(string*);
extern bool string_pop_front(string*);

extern bool string_append(string*, string);
extern bool string_append_raw(string*, const char*);

extern bool string_substr(string, size_t, size_t, string*);

extern void string_swap(string*, string*);
extern int  string_compare(string, string);

extern size_t string_find(string, char);
extern size_t string_find_substr(string, const char*);

#endif


dynstring.c

```
#include "dynstring.h"

#include
#include
#include
#include
#include

bool string_create(string* str)
{
assert(str);
// allocate 1 for '\0'
str->data = malloc(sizeof(char));
if (!str->data) {
return false;
}
str->size = 0;
str->data[0] = '\0';
return true;
}

bool string_create_n(string* str, size_t size)
{
asse

Solution

Improvements

Initializing functions (string_create*)

I think it's a common practice that initializing functions return whole structure element. For example, your implementation

extern bool string_create_n(string* str, size_t size);


should look like this 

extern string * string_create_n(size_t size);


It's also applicable to free/destroy function - this function should destroy whole object (not just data).

Try realloc before malloc

Let's say you calling string_push_back function, so it basically means that you would like to add one more character to the data. Instead of using malloc in a first place I would suggest to try to extend current array by one sizeof(char) using realloc. I also introduced one more variable to the structure - size_t alloc_size keeps actual size of allocated memory. It allows to avoid frequent reallocations events (there is invariant that alloc_size >= size + 1).

Be careful with Memory leaks

Your code contains a lot of problems with memory management. For example

bool string_create(string* str)
{
    assert(str);
    // allocate 1 for '\0'
    str->data = malloc(sizeof(char));
    if (!str->data) {
        return false;
    }
    str->size = 0;
    str->data[0] = '\0';
    return true;
}


Imagine that there is an allocated array in str->data. By making malloc before freeing, you losing link to the first array.

Suggestions

  • I prefer to use const modifier as much as possible. There are


numerous benefits to follow this advice.

  • Thanks to @toby-speight' comment I get to know that identifier should not start from reserved word like str.



TODO List

  • You should remove assert and write meaningful error messages. Instead of returning bool value, use int values which should be macro-defined.



  • dyn_string_push_front and dyn_string_pop_front are implemented not-optimally.



  • Structure dyn_string contains size of the data, so you can skip '\0' ending. It's good idea because, first, you will save memory and, second, you will avoid actions related to '\0' maintenance.



My version

dynstring.c

```
#include "dynstring.h"
#include
#include
#include
#include

dyn_string* dyn_string_alloc(const size_t n)
{
dyn_string *p;
assert(n);
p = (dyn_string *) malloc (sizeof (dyn_string));
assert(p);
p->data = (char ) malloc (n sizeof (char));
assert(p->data);
p->size = 0;
p->data[0] = '\0';
p->alloc_size = n;
return p;
}

void dyn_string_realloc(dyn_string * p, const size_t n)
{
char *c;
assert(p);
if (n > p->alloc_size) {
c = (char ) realloc(p->data, sizeof(char) n);
if(c == 0) {
c = (char ) malloc(sizeof(char) n);
assert(c);
memcpy(c, p->data, sizeof(char) * (p->size + 1));
free(p->data);
p->data = c;
}
p->alloc_size = n;
}
}

dyn_string dyn_string_alloc_raw(const char src)
{
size_t n = strlen(src) + 1;
dyn_string *p = dyn_string_alloc(n);
assert(p);
memcpy(p->data, src, sizeof(char) * n);
p->size = n - 1;
return p;
}

dyn_string dyn_string_alloc_cpy(const dyn_string src)
{
assert(src);
size_t n = src->size + 1;
dyn_string *p = dyn_string_alloc(n);
memcpy(p->data, src->data, sizeof(char) * n);
p->size = src->size;
p->alloc_size = n;
return p;
}

void dyn_string_free(dyn_string * p)
{
assert(p);
free(p->data);
free(p);
}

void dyn_string_cpy(dyn_string dst, const dyn_string src)
{
assert(dst && src);
size_t n = src->size + 1;
if (dst->alloc_size data, src->data, sizeof(char) * n);
dst->size = src->size;
}

void dyn_string_cpy_raw(dyn_string dst, const char src)
{
assert(dst && src);
size_t n = strlen(src) + 1;
if (dst->alloc_size data, src, sizeof(char) * n);
dst->size = n - 1;
}

void dyn_string_set(dyn_string* p, const size_t i, const char c)
{
assert(p && p->size > i);
p->data[i] = c;
}

char dyn_string_get(const dyn_string* p, const size_t i)
{
assert(p->size > i);
return p->data[i];
}

char dyn_string_front(const dyn_string p) {
return p->data;
}

char dyn_string_back(const dyn_string p) {
return p->data + p->size;
}

size_t dyn_string_size(const dyn_string * p) {
return p->size;
}

// erases characters iddn range [i, i+n] including
void dyn_string_erase(dyn_string* p, const size_t i, const size_t n)
{
assert(p && p->size > i);
if (i + n + 1 >= p->size) {
p->size = i;
} else {
p->size -= (n + 1);
memcpy(p->data + i,
p->data + i + n + 1,
sizeof(char) * (p->size - i));
}
p->data[p->size] = '\0';
}

void dyn_string_push_back(dyn_string* p, const char c)
{
assert(p);
if (p->size + 1 alloc_size) {
dyn_string_realloc(p, p->alloc_sizedata[p->size++] = c;
p->data[p->size] = '\0';
}

void dyn_string_push_front(dyn_string* p, const char c)
{
assert(p);
if (p->s

Code Snippets

extern bool string_create_n(string* str, size_t size);
extern string * string_create_n(size_t size);
bool string_create(string* str)
{
    assert(str);
    // allocate 1 for '\0'
    str->data = malloc(sizeof(char));
    if (!str->data) {
        return false;
    }
    str->size = 0;
    str->data[0] = '\0';
    return true;
}
#include "dynstring.h"
#include <stdbool.h>
#include <assert.h>
#include <stdlib.h>
#include <string.h>

dyn_string* dyn_string_alloc(const size_t n)
{
    dyn_string *p;
    assert(n);
    p = (dyn_string *) malloc (sizeof (dyn_string));
    assert(p);
    p->data = (char *) malloc (n * sizeof (char));
    assert(p->data);
    p->size = 0;
    p->data[0] = '\0';
    p->alloc_size = n;
    return p;
}

void dyn_string_realloc(dyn_string * p, const size_t n)
{
    char *c;
    assert(p);
    if (n > p->alloc_size) {
        c = (char *) realloc(p->data, sizeof(char) * n);
        if(c == 0) {
            c = (char *) malloc(sizeof(char) * n);
            assert(c);
            memcpy(c, p->data, sizeof(char) * (p->size + 1));
            free(p->data);
            p->data = c;
        } 
        p->alloc_size = n;
    }
}

dyn_string* dyn_string_alloc_raw(const char* src)
{
    size_t n = strlen(src) + 1;
    dyn_string *p = dyn_string_alloc(n);
    assert(p);
    memcpy(p->data, src, sizeof(char) * n);
    p->size = n - 1;
    return p;
}

dyn_string* dyn_string_alloc_cpy(const dyn_string * src)
{
    assert(src);
    size_t n = src->size + 1;
    dyn_string *p = dyn_string_alloc(n);
    memcpy(p->data, src->data, sizeof(char) * n);
    p->size = src->size;
    p->alloc_size = n;
    return p;
}

void dyn_string_free(dyn_string * p)
{
    assert(p);
    free(p->data);
    free(p);
}

void dyn_string_cpy(dyn_string * dst, const dyn_string * src)
{
    assert(dst && src);
    size_t n = src->size + 1;
    if (dst->alloc_size < n) {
        dyn_string_realloc(dst, n);
    }
    memcpy(dst->data, src->data, sizeof(char) * n);
    dst->size = src->size;
}

void dyn_string_cpy_raw(dyn_string* dst, const char* src)
{
    assert(dst && src);
    size_t n = strlen(src) + 1;
    if (dst->alloc_size < n) {
        dyn_string_realloc(dst, n);
    }
    memcpy(dst->data, src, sizeof(char) * n);
    dst->size = n - 1;
}

void dyn_string_set(dyn_string* p, const size_t i, const char c)
{
    assert(p && p->size > i);
    p->data[i] = c;
}

char dyn_string_get(const dyn_string* p, const size_t i)
{
    assert(p->size > i);
    return p->data[i];
}

char* dyn_string_front(const dyn_string * p) {
    return p->data;
}

char* dyn_string_back(const dyn_string * p) {
    return p->data + p->size;
}

size_t dyn_string_size(const dyn_string * p) {
    return p->size;
}

// erases characters iddn range [i, i+n] including
void dyn_string_erase(dyn_string* p, const size_t i, const size_t n)
{
    assert(p && p->size > i);
    if (i + n + 1 >= p->size) {
        p->size = i;
    } else {
        p->size -= (n + 1);
        memcpy(p->data + i,
               p->data + i + n + 1,
               sizeof(char) * (p->size - i));
    } 
    p->data[p->size] = '\0';
}

void dyn_string_push_back(dyn_string* p, const char c)
{
    assert(p);
    if (p->size + 1 <= p->alloc_size) {
        dyn_string_realloc(p, p->alloc_size<<1);
    }
    p->data[p->size++] = c;
    p->data[p->size]
#ifndef DYNSTRING_H
#define DYNSTRING_H

#include <stddef.h>

typedef struct {
    char* data;
    size_t size;
    size_t alloc_size;
} dyn_string;

extern dyn_string* dyn_string_alloc(const size_t);
extern void dyn_string_realloc(dyn_string*, const size_t);
extern dyn_string* dyn_string_alloc_raw(const char*);
extern dyn_string* dyn_string_alloc_cpy(const dyn_string*);

extern void dyn_string_free(dyn_string*);

extern void dyn_string_cpy(dyn_string*, const dyn_string*);
extern void dyn_string_cpy_raw(dyn_string*, const char*);

extern void dyn_string_set(dyn_string*, const size_t, const char);
extern char dyn_string_get(const dyn_string*, const size_t);

extern char* dyn_string_front(const dyn_string*);
extern char* dyn_string_back(const dyn_string*);

extern size_t dyn_string_size(const dyn_string*);
extern void dyn_string_erase(dyn_string*, const size_t, const size_t);

extern void dyn_string_push_back(dyn_string*, const char);
extern void dyn_string_push_front(dyn_string*, const char);

extern char dyn_string_pop_back(dyn_string*);
extern char dyn_string_pop_front(dyn_string*);

extern void dyn_string_append(dyn_string*, const dyn_string*);
extern void dyn_string_append_raw(dyn_string*, const char*);

extern void dyn_string_substr(dyn_string*, const dyn_string *, const size_t, const size_t);

extern void dyn_string_swap(dyn_string*, dyn_string*);
extern int  dyn_string_compare(const dyn_string*, const dyn_string*);

extern size_t dyn_string_find(const dyn_string*, const char);
extern size_t dyn_string_find_substr(const dyn_string*, const char*);

#endif

Context

StackExchange Code Review Q#159010, answer score: 2

Revisions (0)

No revisions yet.