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

Dynamic Strings in C

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

Problem

I wrote this code for dynamic strings and would like to know what mistakes I've made.

It's just a struct that gets filled on the first call to ds_allocate and freed on the first call to ds_free. It uses memcpy for concatenation and supports a few basic operations.

Here's the header

#include 
#include 
#include 
#include 

//Size and expansion
#define MULTIPLIER 1.00 //add 100% every time
#define FIXED_STEP 0 //overrides multiplier
#define STARTING_SIZE 24

//Can set other memory functions
#define allocate malloc
#define deallocate free
#define reallocate realloc

//Main structure
typedef struct {
    char *content;
    char *position;
    char *end; //1 past the end
} Dynamic_String;


And the code so far:

```
static inline size_t max(size_t x, size_t y)
{
return (x > y) ? x : y;
}
static inline size_t min(size_t x, size_t y)
{
return (x 0) ? custom_size : STARTING_SIZE;

char *start = allocate(size);
if(start == NULL){
return NULL;
}

ds->content = ds->position = start;
ds->end = start + size;
*start = '\0';

return start;
}

void ds_free(Dynamic_String *ds)
{
deallocate(ds->content);
}

//Keep memory allocated, clear contents
void ds_clear(Dynamic_String *ds)
{
ds->position = ds->content;
*ds->position = '\0';
}

//If the content is manipulated without using these functions, but the memory
//allocated is the same and there's a '\0', it corrects the string position.
//Otherwise it writes a new '\0' at the end and returns NULL. The string
//should be usable after calling this function.
char ds_fix(Dynamic_String broken)
{
assert(broken->content != NULL);

broken->position = memchr(broken->content, '\0', ds_capacity(broken));

if(broken->position == NULL){
broken->position = broken->end - 1;
*broken->position = '\0';
return NULL;
}
return broken->position;
}

//Equivalent of strlen
size_t ds_length(const Dynamic_String *ds)
{
return ds->position

Solution

The code is generally nice, so my comments are really just picking up crumbs.

I'm not comfortable with your use of asserts to catch errors that could
legitimately occur at runtime. I consider asserts to be for asserting logical
impossibilities, not for catching runtime errors.

Function expand is equivalent to expand_by_at_least(ds, 1)

Long parameter names destination and source could be abbreviated dst and
src with no loss.

Is giving the user a read-write (ie. non-const) pointer to the private string
a good idea (functions returning char*)? And is the position returned
consistent (think not - sometimes the new end of string sometimes the old).

In ds_compare (ds_compare_n), why not just return the strcmp (strncmp)
of the two strings?

I don't like the loop in ds_append_cstring. A simple loop copying and
checking for destination full, expanding as necessary would be simpler.

Edit: Do you mind giving me an example of what assert errors could legitimately
occur at runtime?

In the context of your code, I have no problem with asserting non-null
pointers, but asserting for non-zero string length seems unsafe. There are
many times when a string could have a zero length but the assert says that
users of your code must guarantee that it is always a design fault in their
code for a nul string to pass into your functions. Not only is this
unreasonable, it is is unnecessary as you can easily handle such cases. BTW
don't assume that mallocing 0 is equivalent to free (it isn't) - handle a
zero request by allocating at least 1 byte.

As an example of testing a logical impossibilty consider some code that does:

expected length = calculate expected length of string
allocate memory
real length = create string in memory
assert(real length = expected length)


The assert tells you whether your algorithms are consistent/correct

Code Snippets

expected length = calculate expected length of string
allocate memory
real length = create string in memory
assert(real length = expected length)

Context

StackExchange Code Review Q#38236, answer score: 5

Revisions (0)

No revisions yet.