5
\$\begingroup\$

I have wrote a smart string container implementation for use in my application, but as I'am not such professional C programmer I have doubts about is I'am did it right and is there ways how to improve it and make it better?


#include <assert.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <wchar.h>

#define DEFAULT_CAPACITY 16

#define pntstrt_deftypeutil_strlen(T, fn) size_t (* pntstrt_typeutil_strlen$$##T)(const T*) = fn;
#define pntstrt_deftypeutil_strcpy(T, fn) T* (* pntstrt_typeutil_strcpy$$##T)(T*, const T*) = fn;

#define PNTSTRT_DECLARE_TYPE(T) \
typedef struct __pntstrt$$##T pntstrt$$##T; \
struct __pntstrt$$##T { \
  T *data; \
  size_t capacity; \
  size_t length; \
}; \
void pntstrt_init$$##T(pntstrt$$##T *str) \
{ \
  str->data = calloc(DEFAULT_CAPACITY, sizeof(T)); \
  *str->data = (T)0; \
  str->capacity = DEFAULT_CAPACITY; \
  str->length = 0; \
} \
pntstrt$$##T pntstrt_initrv$$##T() \
{ \
  pntstrt$$##T str; \
  pntstrt_init$$##T(&str); \
  return str; \
} \
void pntstrt_make$$##T(pntstrt$$##T *str, T *buf) \
{ \
  size_t len = pntstrt_typeutil_strlen$$##T(buf); \
  size_t newcap = ((len + 1) / DEFAULT_CAPACITY + 1) * DEFAULT_CAPACITY; \
  if (newcap != str->capacity) { \
    T *pData = realloc(str->data, newcap * sizeof(T)); \
    if (!pData) { \
      assert(0); \
      return; \
    } \
    str->data = pData; \
    str->capacity = newcap; \
  } \
  pntstrt_typeutil_strcpy$$##T(str->data, buf); \
  str->data[len] = (T)0; \
  str->length = len; \
} \
pntstrt$$##T pntstrt_makerv$$##T(T *buf) \
{ \
  pntstrt$$##T str; \
  pntstrt_init$$##T(&str); \
  pntstrt_make$$##T(&str, buf); \
  return str; \
} \
void pntstrt_append$$##T(pntstrt$$##T *str1, T *str2) \
{ \
  size_t newlen = str1->length + pntstrt_typeutil_strlen$$##T(str2); \
  size_t newcap = ((newlen + 1) / DEFAULT_CAPACITY + 1) * DEFAULT_CAPACITY; \
  if (newcap != str1->capacity) { \
    T *pData = realloc(str1->data, newcap * sizeof(T)); \
    if (!pData) { \
      assert(0); \
      return; \
    } \
    str1->data = pData; \
    str1->capacity = newcap; \
  } \
  pntstrt_typeutil_strcpy$$##T(&str1->data[str1->length], str2); \
  str1->data[newlen] = (T)0; \
  str1->length = newlen; \
} \
void pntstrt_concat$$##T(pntstrt$$##T *str1, pntstrt$$##T *str2) \
{ \
  pntstrt_append$$##T(str1, str2->data); \
} \
const T* pntstrt_cstr$$##T(pntstrt$$##T *str) \
{ \
  return str->data; \
} \
size_t pntstrt_len$$##T(pntstrt$$##T *str) \
{ \
  return str->length; \
} \
void pntstrt_destroy$$##T(pntstrt$$##T *str) \
{ \
  free(str->data); \
  str->data = NULL; \
  str->length = 0; \
}

#define PNTPATHUTIL_DECLARE_TYPE(T) \
void pntpathutil_add_separator$$##T(pntstrt$$##T *str) \
{ \
  if (str->length && str->data[str->length-1] == (T)'\\') { \
    return; \
  } \
  pntstrt_append$$##T(str, (T[]){(T)'\\', (T)0}); \
} \

#define pntstrt(T) pntstrt$$##T
#define pntstrt_init(T) pntstrt_init$$##T
#define pntstrt_initrv(T) pntstrt_initrv$$##T
#define pntstrt_make(T) pntstrt_make$$##T
#define pntstrt_set(T) pntstrt_make$$##T
#define pntstrt_makerv(T) pntstrt_makerv$$##T
#define pntstrt_append(T) pntstrt_append$$##T
#define pntstrt_concat(T) pntstrt_concat$$##T
#define pntstrt_cstr(T) pntstrt_cstr$$##T
#define pntstrt_len(T) pntstrt_len$$##T
#define pntstrt_destroy(T) pntstrt_destroy$$##T

pntstrt_deftypeutil_strlen(char, strlen)
pntstrt_deftypeutil_strcpy(char, strcpy)
PNTSTRT_DECLARE_TYPE(char)
    
pntstrt_deftypeutil_strlen(wchar_t, wcslen)
pntstrt_deftypeutil_strcpy(wchar_t, wcscpy)
PNTSTRT_DECLARE_TYPE(wchar_t) 

#define pntstr pntstrt(char)
#define pntstr_init pntstrt_init(char)
#define pntstr_initrv pntstrt_initrv(char)
#define pntstr_make pntstrt_make(char)
#define pntstr_makerv pntstrt_makerv(char)
#define pntstr_append pntstrt_append(char)
#define pntstr_concat pntstrt_concat(char)
#define pntstr_cstr pntstrt_cstr(char)
#define pntstr_len pntstrt_len(char)
#define pntstr_destroy pntstrt_destroy(char)

#define pntstrw pntstrt(wchar_t)
#define pntstrw_init pntstrt_init(wchar_t)
#define pntstrw_initrv pntstrt_initrv(wchar_t)
#define pntstrw_make pntstrt_make(wchar_t)
#define pntstrw_makerv pntstrt_makerv(wchar_t)
#define pntstrw_append pntstrt_append(wchar_t)
#define pntstrw_concat pntstrt_concat(wchar_t)
#define pntstrw_cstr pntstrt_cstr(wchar_t)
#define pntstrw_len pntstrt_len(wchar_t)
#define pntstrw_destroy pntstrt_destroy(wchar_t)
    
#define pntpathutil_add_separator(T) pntpathutil_add_separator$$##T

PNTPATHUTIL_DECLARE_TYPE(char)
PNTPATHUTIL_DECLARE_TYPE(wchar_t)

#define pathutil_add_separator pntpathutil_add_separator(char)
#define pathutilw_add_separator pntpathutil_add_separator(wchar_t)
    
int main(void)
{
    pntstrw str1;
    pntstrw str2;
    pntstrw_init(&str1);
    pntstrw_init(&str2);

    pntstrw_make(&str1, L"Hello");
    pntstrw_make(&str2, L", World!");
    
    pntstrw_concat(&str1, &str2);
    pntstrw_append(&str1, L" ghi");
    
    pathutilw_add_separator(&str1);
    pntstrw_append(&str1, L" jklm");
    pathutilw_add_separator(&str1);
    
    wprintf(L"str1.len: %ld, str2.len: %ld\n", pntstrw_len(&str1), pntstrw_len(&str2));
    wprintf(L"str1.cap: %ld, str2.cap: %ld\n", str1.capacity, str2.capacity);
    
    wprintf(pntstrw_cstr(&str1));
    
    pntstrw_destroy(&str1);
    pntstrw_destroy(&str2);
    return 0;
}
```
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

Lack of documentation

Code deserve comments explaining what this code set does.

Allow initialization at defintion time

Consider another form for pntstrw_init():

// pntstrw str1;
// pntstrw_init(&str1);
pntstrw str1 = pntstrw_init();

Design

I am not much of a fan of huge .h files implementing code and would rather use a tidy .h file and accompanying .c file - but to each his own.

Growth

It looks like memory growth is linear. I recommend some closer to exponential like doubling to avoid slowness.

Name pollution

DEFAULT_CAPACITY is a surprising name to find in amongst pntstrt_ code and thus a collision candidate with other code. Perhaps pntstrt_DEFAULT_CAPACITY or the like.

Separate into a .h file

Put the pntstrt_ code in its own file and main() in another to show how to use this code in larger applications.

Double a string?

Consider pntstrw_concat(&str1, &str1); (str1 used 2x). To support this, pntstrt_append() should not reallocate before the concatenating is made. Instead:

  • Allocate for new space.
  • Copy str1 and str2 into it.
  • Then free str1 former buffer.

Code after pntstrw_destroy()

I'd expect .capacity to be set to 0 in pntstrw_destroy().

Consider the effect of calling functions after pntstrw_destroy(). Do you want undefined or well defined behavior? Many functions presently incur UB with str->data == NULL.

If UB is OK, no need for str->data = NULL; str->length = 0;.

Mis-matched printf() specifier/type

"%ld" is for long.
"%zu" is for size_t.

//                  %zu                        size_t    
wprintf(L"str1.len: %ld, str2.len: %ld\n", pntstrw_len(&str1),  ... 

Why \\?

pntpathutil_add_separator() uses '\\' for a path separator. The best separator is usually OS dependent. More likely '/' is a better choice if fixed.

Missing use of const

Select functions deserve const like pntstrt_cstr.

// const T* pntstrt_cstr$$##T(pntstrt$$##T *str)
const T* pntstrt_cstr$$##T(const pntstrt$$##T *str)

Default size

Rather than DEFAULT_CAPACITY as 16, consider 1. I plan for large uses of objects and then in that case, many are empty and default of 16 is wasteful. It comes down to Time vs. Memory. One of those is finite. A default of 16 makes sense after some thing more than the initial amount is needed.

\$\endgroup\$
2
  • \$\begingroup\$ Wow, this is amazing! You pointed out problems that I didn't even seen in advance. You answer is great, I don't know how to thank, you deserves more. And if you please, there another problem that was not covered but I'am uncertainty about: is that string functions workaround with pntstrt_deftypeutil_*-like macros is a bad practice, isn't it? \$\endgroup\$
    – Alex
    Mar 17, 2022 at 14:19
  • \$\begingroup\$ @Alex, "string functions workaround with pntstrt_deftypeutil_" I found unclear so lack of clarity is a negative. It apparently forms a non-static global object that would conflict with multiple *.c files using these definitions, another minus. \$\endgroup\$ Mar 17, 2022 at 16:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.