String lib in C

Question

I am new at C programming, so I would appreciate it if you could highlight the problems in my code. My biggest concern is wrong memory allocation.

There are some code snippets, where I repeat my code. Is there another way to get rid of them?

Github repo link.

sstream.h

#pragma once
#include <stdlib.h>
typedef struct sstream sstream;
struct sstream {
 char* buffer;
 unsigned int length;
 unsigned int capacity;
};
sstream* sstream_create(char* str);
void sstream_free(sstream* str);
char* sstream_get(sstream* str);
unsigned int sstream_lenght(sstream* str);
unsigned int sstream_capacity(sstream* str);
char sstream_atIndex(sstream* str, int idx);
unsigned int sstream_findChar(sstream* str, char c);
void sstream_print(sstream* str);
sstream* sstream_copy(sstream* str);
void sstream_pushFrontChar(sstream* str, char c);
void sstream_pushFrontStr(sstream* str, sstream* new);
void sstream_pushBackChar(sstream* str, char c);
void sstream_pushBackStr(sstream* str, sstream* new);
void sstream_insertStr(sstream* str, unsigned int idx, sstream* new);
void sstream_popFront(sstream* str);
void sstream_popBack(sstream* str);
void sstream_erase(sstream* str, unsigned int idx, unsigned int len);
void sstream_replace(sstream* str, unsigned int idx, unsigned int len, sstream* new);
sstream* sstream_substring(sstream* str, unsigned int idx, unsigned int len);

sstream.c

#include "libs/sstream.h"
#include <string.h>
#include <stdio.h>
sstream* sstream_create(char* str) {
 sstream* result = malloc(sizeof(sstream));
 result->length = strlen(str);
 result->capacity = result->length * 2;
 result->buffer = malloc(result->capacity+1);
 memcpy(result->buffer,str,result->length);
 result->buffer[result->length] = '0円';
 return result;
}
void sstream_free(sstream* str) {
 free(str->buffer);
 str->buffer = NULL;
}
char* sstream_get(sstream* str) {
 return str->buffer;
}
unsigned int sstream_lenght(sstream* str) {
 return str->length;
}
unsigned int sstream_capacity(sstream* str) {
 return str->capacity;
}
char sstream_atIndex(sstream* str, int idx) {
 return str->buffer[idx];
}
unsigned int sstream_findChar(sstream* str, char c) {
 return strchr(str->buffer, c) - str->buffer;
}
void sstream_print(sstream* str) {
 printf("%s",sstream_get(str));
}
sstream* sstream_copy(sstream* str) {
 return sstream_create(str->buffer);
}
void sstream_pushFrontChar(sstream* str, char c) {
 if(str->capacity < str->length + 2) { //+1 c, +1 0円
 char* temp = realloc(sstream_get(str),str->capacity*2);
 str->capacity = str->capacity * 2;
 str->buffer = temp;
 }
 memmove(&str->buffer[1],&str->buffer[0],str->length+1);
 str->buffer[0] = c;
 str->length = str->length + 1;
 str->buffer[str->length] = '0円';
}
void sstream_pushFrontStr(sstream* str, sstream* new) {
 if(str->capacity < (str->length + new->length + 1)) { //+1 0円
 char* temp = realloc(sstream_get(str),str->capacity * 2);
 str->capacity = str->capacity * 2;
 str->buffer = temp;
 if(str->capacity < (str->length + new->length + 1)) {
 sstream_pushFrontStr(str,new);
 return;
 }
 }
 memmove(&str->buffer[new->length],&str->buffer[0],str->length);
 memmove(&str->buffer[0],&new->buffer[0],new->length);
 str->length = str->length + new->length;
 str->buffer[str->length] = '0円';
}
void sstream_pushBackChar(sstream* str, char c) {
 if(str->capacity < str->length + 2) { //+1 c, +1 0円
 char* temp = realloc(sstream_get(str),str->capacity*2);
 str->capacity = str->capacity * 2;
 str->buffer = temp;
 }
 str->length = str->length + 1;
 str->buffer[str->length - 1] = c;
 str->buffer[str->length] = '0円';
}
void sstream_pushBackStr(sstream* str, sstream* new) {
 if(str->capacity < (str->length + new->length + 1)) { //+1 0円
 char* temp = realloc(sstream_get(str),str->capacity * 2);
 str->capacity = str->capacity * 2;
 str->buffer = temp;
 if(str->capacity < (str->length + new->length + 1)) {
 sstream_pushBackStr(str,new);
 return;
 }
 }
 memmove(&str->buffer[str->length],new->buffer,new->length);
 str->length = str->length + new->length;
 str->buffer[str->length] = '0円';
}
void sstream_insertStr(sstream* str, unsigned int idx, sstream* new) {
 if(str->capacity < (str->length + new->length + 1)) { //+1 0円
 char* temp = realloc(sstream_get(str),str->capacity * 2);
 str->capacity = str->capacity * 2;
 str->buffer = temp;
 if(str->capacity < (str->length + new->length + 1)) {
 sstream_insertStr(str,idx,new);
 return;
 }
 }
 memmove(str->buffer,str->buffer,idx);
 memmove(&str->buffer[idx + new->length],&str->buffer[idx],str->length - idx);
 str->length = str->length + new->length;
 memmove(&str->buffer[idx],new->buffer,new->length);
 str->buffer[str->length] = '0円';
}
void sstream_popFront(sstream* str) {
 memmove(&str->buffer[0],&str->buffer[1],str->length-1);
 str->length = str->length - 1;
 str->buffer[str->length] = '0円';
}
void sstream_popBack(sstream* str) {
 memmove(&str->buffer[str->length],&str->buffer[str->length+1],str->length - 1);
 str->length = str->length - 1;
 str->buffer[str->length] = '0円';
}
void sstream_erase(sstream* str, unsigned int idx, unsigned int len) {
 memmove(&str->buffer[idx],&str->buffer[idx + len], str->length - idx);
 str->length = str->length - len;
 if(str->capacity > (str->length * 16)) { // if to big
 str->capacity = str->length * 2 + 1;
 str->buffer = realloc(str->buffer,str->capacity);
 }
 str->buffer[str->length] = '0円';
}
void sstream_replace(sstream* str, unsigned int idx, unsigned int len, sstream* new) {
 if(str->capacity < (str->length + new->length + 1)) { //+1 0円
 char* temp = realloc(sstream_get(str),str->capacity * 2);
 str->capacity = str->capacity * 2;
 str->buffer = temp;
 if(str->capacity < (str->length + new->length + 1)) {
 sstream_replace(str,idx,len,new);
 return;
 }
 }
 memmove(str->buffer,str->buffer,idx);
 memmove(&str->buffer[idx],new->buffer,idx + new->length);
 memmove(&str->buffer[idx + new->length], &str->buffer[idx + len], str->length + new->length - len);
 str->length = str->length + new->length - len;
 str->buffer[str->length] = '0円';
}
sstream* sstream_substring(sstream* str, unsigned int idx, unsigned int len) {
 sstream* result = malloc(sizeof(sstream));
 result->length = len;
 result->capacity = result->length * 2;
 result->buffer = malloc(result->capacity + 1);
 memcpy(result->buffer,&str->buffer[idx],result->length);
 result->buffer[result->length] = '0円';
 return result;
}

• Toby Speight: If your biggest concern is the memory correctness, it would be very helpful to show the test suite too (presumably you've run that under Valgrind or similar?) - then we'd be able to point out testing that you've missed.
• ekato: Oh okay, I do not know about this program.

Answer 1

I don't see any need for the header to include <stdlib.h>. It should compile just fine without that, and only the implementation needs to include it.

unsigned int length;
unsigned int capacity;

However, perhaps we do want <stdint.h>, so that we can use size_t for these values (that's a more natural choice, and is what we get back from strlen(), for example).

It's good that you include sstream.h first in the implementation - that's a good test that the header is complete, and usable without prerequisites.

There's a severe lack of robustness when allocating memory. Any memory allocation can fail, and this code runs into undefined behaviour whenever malloc() or realloc() returns a null pointer. As an example:

sstream* sstream_create(char* str)
{
 sstream* result = malloc(sizeof *result);
 if (!result) {
 return result;
 }
 result->length = strlen(str);
 result->capacity = result->length * 2;
 result->buffer = malloc(result->capacity+1);
 if (!result->buffer) {
 free(result);
 return NULL;
 }
 memcpy(result->buffer, str, result->length);
 result->buffer[result->length] = '0円';
 return result;
}

Those two if statements are essential for correctness in low-memory situations.

Special care needs to be taken with realloc(). This is a typical anti-pattern:

 str->buffer = realloc(str->buffer,str->capacity);

If the realloc() fails, str->buffer will be null, and we no longer have a pointer to the memory it previously pointed to - that's a memory leak.

 char *new_buf = realloc(str->buffer,str->capacity);
 if (!new_buf) {
 /* handle the error here */
 } else {
 str->buffer = new_buf;
 }

We can make the code a little more readable by using += and ++ rather than repeating the variable. E.g.

str->length = str->length + 1;
str->length = str->length + new->length;

can be replaced by

++str->length;
str->length += new->length;

Trivial typo: sstream_lenght should be sstream_length.