Spun article reader

Question

This is a simple GTK+ program that takes a spun article as input and shows a random output every time the user clicks the "Spin" button.

It supports many levels of nested spinning like:

The {car|automobile} is {{very |}fine|{really {pretty |}|}cool|all right}

I tested it and it seems to be correct, but I would like to receive any suggestions on how to improve the code, specially if there's some bug I didn't notice.

#include <gtk/gtk.h>
#include <stdlib.h>
#include "dynamic_string.h"
#include <time.h>

#define SUCCESS 1
#define ERROR 0

GtkWidget *output_text;

size_t get_size(const char *src)
{
    const char *start = src;
    while(*++src && *src != '}')
        if(*src == '{')
            src += get_size(src) - 1;

    return (src - start) + 1;
}

size_t get_count(const char *src)
{
    size_t count = 1;
    while(*++src){
        if(*src == '}')
            break;

        else
        if(*src == '|')
            ++count;

        else
        if(*src == '{')
            src += get_size(src) - 1;
    }

    return count;
}

const char *get_word_n(const char *start, size_t n)
{
    size_t count = 0;
    while(*++start){
        if(count == n)
            return start;

        if(*start == '}')
            break;

        else
        if(*start == '|')
            ++count;

        else
        if(*start == '{')
            start += get_size(start) - 1;
    }

    puts("Invalid format or there are not as many words as you expected.");
    return NULL;
}

int choose_one(const char *src, Dynamic_String *dest)
{
    size_t count = get_count(src);
    const char *str = get_word_n(src, rand() % count);

    if(str == NULL)
        return ERROR;

    while(*str && *str != '}' && *str != '|'){
        if(*str == '{'){
            if(choose_one(str, dest) == ERROR)
                return ERROR;

            str += get_size(str) - 1;
        }
        else
        if(ds_push_back(dest, *str) == DS_ERROR)
            exit(1);

        ++str;
    }

    return SUCCESS;
}

gboolean spin(GtkButton *button, GdkEvent *event, GtkWidget *input)
{
    Dynamic_String output;
    ds_allocate(&output, 4095);

    GtkTextBuffer *buffer = gtk_text_view_get_buffer(GTK_TEXT_VIEW(input));
    GtkTextIter start, end;

    gtk_text_buffer_get_bounds(buffer, &start, &end);
    char *content = gtk_text_buffer_get_text(buffer, &start, &end, FALSE);

    for(size_t i = 0; content[i] != '\0'; ++i){
        if(content[i] == '{'){
            if(choose_one(&content[i], &output) == ERROR)
                goto end;
            i += get_size(&content[i]) - 1;
        }
        else
        if(ds_push_back(&output, content[i]) == DS_ERROR)
            exit(1);
    }

    buffer = gtk_text_view_get_buffer(GTK_TEXT_VIEW(output_text));
    gtk_text_buffer_set_text(buffer, output.cstring, -1);

    end:
    free(content);
    ds_free(&output);

    return FALSE;
}

int main(int argc, char **argv)
{
    srand(time(NULL));
    gtk_init(&argc, &argv);

    /* Main window */
    GtkWidget *window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
    gtk_window_set_title(GTK_WINDOW(window), "Article Viewer");
    gtk_container_set_border_width(GTK_CONTAINER(window), 0);
    gtk_window_set_default_size(GTK_WINDOW(window), 1280, 720);
    g_signal_connect(window, "destroy", G_CALLBACK(gtk_main_quit), NULL);

    /* Text container, scrolling */
    GtkWidget *scrolled_window = gtk_scrolled_window_new(NULL, NULL);
    gtk_scrolled_window_set_policy(    GTK_SCROLLED_WINDOW(scrolled_window),
                                    GTK_POLICY_AUTOMATIC,
                                    GTK_POLICY_AUTOMATIC                );
    gtk_widget_set_hexpand(scrolled_window, TRUE);
    gtk_widget_set_vexpand(scrolled_window, TRUE);

    /* Text container */
    GtkWidget *input_text = gtk_text_view_new();
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(input_text),
                                            GTK_TEXT_WINDOW_TOP,
                                            10                            );
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(input_text),
                                            GTK_TEXT_WINDOW_BOTTOM,
                                            10                            );
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(input_text),
                                            GTK_TEXT_WINDOW_LEFT,
                                            10                            );
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(input_text),
                                            GTK_TEXT_WINDOW_RIGHT,
                                            10                            );
    gtk_text_view_set_wrap_mode(GTK_TEXT_VIEW(input_text), GTK_WRAP_WORD_CHAR);

    /* Add text to container */
    gtk_container_add(GTK_CONTAINER(scrolled_window), input_text);

    /* Append input tab */
    GtkWidget *notebook = gtk_notebook_new();
    GtkWidget *label = gtk_label_new("Input");
    gtk_notebook_append_page(GTK_NOTEBOOK(notebook), scrolled_window, label);

    /* Output container */
    GtkWidget *box = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0);
    gtk_box_set_homogeneous(GTK_BOX(box), FALSE);

    /* Spin button */
    GtkWidget *button = gtk_button_new_with_label("Spin");
    g_signal_connect(button, "button-release-event", G_CALLBACK(spin), input_text);

    /* Add button to box */
    gtk_box_pack_start(GTK_BOX(box), button, FALSE, FALSE, 0);

    /* Output scrolling */
    scrolled_window = gtk_scrolled_window_new(NULL, NULL);
    gtk_scrolled_window_set_policy(    GTK_SCROLLED_WINDOW(scrolled_window),
                                    GTK_POLICY_AUTOMATIC,
                                    GTK_POLICY_AUTOMATIC                );
    gtk_widget_set_hexpand(scrolled_window, TRUE);
    gtk_widget_set_vexpand(scrolled_window, TRUE);

    /* Output text */
    output_text = gtk_text_view_new();
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(output_text),
                                            GTK_TEXT_WINDOW_TOP,
                                            10                            );
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(output_text),
                                            GTK_TEXT_WINDOW_BOTTOM,
                                            10                            );
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(output_text),
                                            GTK_TEXT_WINDOW_LEFT,
                                            10                            );
    gtk_text_view_set_border_window_size(    GTK_TEXT_VIEW(output_text),
                                            GTK_TEXT_WINDOW_RIGHT,
                                            10                            );
    gtk_text_view_set_wrap_mode(GTK_TEXT_VIEW(output_text), GTK_WRAP_WORD_CHAR);

    /* Add output text to scrolling */
    gtk_container_add(GTK_CONTAINER(scrolled_window), output_text);

    /* Add scrolled window to box */
    gtk_box_pack_start(GTK_BOX(box), scrolled_window, TRUE, TRUE, 0);

    /* Append output tab */
    label = gtk_label_new("Output");
    gtk_notebook_append_page(GTK_NOTEBOOK(notebook), box, label);

    gtk_container_add(GTK_CONTAINER(window), notebook);

    gtk_widget_show_all(window);
    gtk_main();

    return 0;
}

dynamic_string.h

#ifndef DYNAMIC_STRING_H
#define DYNAMIC_STRING_H

#include <stdbool.h>
#include <stdlib.h>

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

//Return codes
#define DS_SUCCESS 1
#define DS_ERROR 0

//ds_fix return codes
#define DS_FIX_FULLY_WORKED DS_SUCCESS
#define DS_FIX_OVERWRITTEN DS_ERROR

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

//Main structure
#ifndef DS_IMPLEMENTATION
typedef struct {
    char *cstring;
    char *position;
    char *end; //1 past the end
} Dynamic_String;
#else
typedef struct {
    char *content;
    char *position;
    char *end; //1 past the end
} Dynamic_String;
#endif

//Allocate a dynamic string container and call ds_allocate
Dynamic_String *ds_new(size_t custom_size);

//Free contents and container
void ds_delete(Dynamic_String *ds);

//Allocate initial space and set the structure members, return pointer to
//newly allocated memory. Available for use is custom_size. It doesn't
//allocate space for the structure itself
int ds_allocate(Dynamic_String *ds, size_t custom_size);

void ds_free(Dynamic_String *ds);

//Keep memory allocated, clear contents
void ds_clear(Dynamic_String *ds);

/* 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. The string should be usable after 
calling this function. */
int ds_fix(Dynamic_String *broken);

//Equivalent to strlen
size_t ds_length(const Dynamic_String *ds);

//Total memory allocated
size_t ds_capacity(const Dynamic_String *ds);

//Space available, accounts for '\0'
size_t ds_space(const Dynamic_String *ds);

bool ds_is_empty(const Dynamic_String *ds);

bool ds_is_full(const Dynamic_String *ds);

//Resize memory and update the structure.
int ds_resize(Dynamic_String *ds, size_t new_size);

//Allocate more bytes
int ds_reserve(Dynamic_String *ds, size_t amount);

//Deallocate part of memory
int ds_shrink(Dynamic_String *ds, size_t amount);

//Reduce allocated storage so it's just enough for the current content
int ds_shrink_to_fit(Dynamic_String *ds);


//Push character to the end of string, return pointer to it
int ds_push_back(Dynamic_String *ds, int c);

//Append one dynamic string to another, return content position
int ds_append(Dynamic_String *destination, const Dynamic_String *source);

//Crop out part of the string
void ds_crop(Dynamic_String *ds, size_t total);

//Append at most n characters from source to destination. Return location of
//source inside destination 
int ds_append_n(    Dynamic_String *destination,
                    const Dynamic_String *source, 
                    size_t max                        );

//Compare two dynamic strings and return  0 if equal, positive if first
//differing character is greater on str1 or negative if smaller
int ds_compare(const Dynamic_String *str1, const Dynamic_String *str2);

//Compare up to n characters
int ds_compare_n(    const Dynamic_String *str1, 
                    const Dynamic_String *str2, 
                    size_t max                    );

//Swap one dynamic string for another
void ds_swap(Dynamic_String *ds1, Dynamic_String *ds2);

///////////////////////
////// Functions to work with dynamic and regular strings

//Takes an already allocated regular string and put it into a container. Making
//it a normal dynamic string. Container must not hold an allocated string or
//there will be memory leaks. 
void ds_from_cstring(Dynamic_String *container, char *c_string);

//Reduce storage to the minimum, and return content as C string
char *ds_to_cstring(Dynamic_String *ds);

//Join a list of Dynamic_Strings and return their content as C string
//The list must be delimited by NULL
char *ds_join_list_to_cstring(Dynamic_String **ds_list);

//Return an allocated copy of Dynamic_String content
char *ds_content_copy(const Dynamic_String *ds);

//Append regular C string to Dynamic_String
int ds_append_cstring(Dynamic_String *destination, const char *c_string);

//Appending a C string of known size is faster
int ds_append_cstring_by_length(    Dynamic_String *destination,
                                    const char *c_string,
                                    size_t length                    );

#endif

dynamic_string.c

#define DS_IMPLEMENTATION

#include <string.h>
#include <stdlib.h>
#include "dynamic_string.h"


////////////////////////////
/////////    Internal methods
///////////////////////////
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 < y) ? x : y;
}


//Compile a different function set depending on settings
#if FIXED_STEP > 0

//Expand according to multiplier or fixed step
static inline int expand(Dynamic_String *ds)
{
    return ds_reserve(ds, FIXED_STEP);
}

//Expand by at least a minimum value
static inline int expand_by_at_least(Dynamic_String *ds, size_t minimum)
{    
    return ds_reserve(ds, max(FIXED_STEP, minimum));
}

#else //Multiplier will be used

//Expand according to multiplier or fixed step
static inline int expand(Dynamic_String *ds)
{
    return ds_reserve(ds, ds_capacity(ds) * MULTIPLIER);
}

static inline int expand_by_at_least(Dynamic_String *ds, size_t minimum)
{    
    return ds_reserve(ds, max(ds_capacity(ds) * MULTIPLIER, minimum));
}

#endif


////////////////////////////
/////////    Public methods
///////////////////////////

//Allocate container and space for characters
Dynamic_String *ds_new(size_t custom_size)
{
    Dynamic_String *temp = allocate(sizeof(Dynamic_String));
    if(temp == NULL)
        return NULL;

    if(ds_allocate(temp, custom_size) == DS_ERROR){
        free(temp);
        return NULL;
    }

    return temp;
}

void ds_delete(Dynamic_String *ds)
{
    ds_free(ds);
    deallocate(ds);
}

/* Allocate initial space and set the structure members, return pointer to newly
allocated memory. Available for use is custom_size. It doesn't allocate
space for the structure itself */
int ds_allocate(Dynamic_String *ds, size_t custom_size)
{
    size_t size = (custom_size > 0) ? custom_size : STARTING_SIZE;

    char *start = allocate(size + 1);
    if(start == NULL)
        return DS_ERROR;

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

    return DS_SUCCESS;
}

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. The string should be usable after 
calling this function. */
int ds_fix(Dynamic_String *broken)
{    
    broken->position = memchr(broken->content, '\0', ds_capacity(broken));

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

    return DS_FIX_FULLY_WORKED;
}

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

//Total memory allocated
size_t ds_capacity(const Dynamic_String *ds)
{
    return ds->end - ds->content;
}

//Space available, accounts for '\0'
size_t ds_space(const Dynamic_String *ds)
{
    return ds->end - ds->position - 1;
}

bool ds_is_empty(const Dynamic_String *ds)
{
    return ds->position == ds->content;
}

bool ds_is_full(const Dynamic_String *ds)
{
    return ds_space(ds) == 0;
}

//Resize memory and update the structure. Return new location
int ds_resize(Dynamic_String *ds, size_t new_size)
{    
    //Location might change
    size_t position_offset = ds_length(ds);

    //Make sure there's at least 1 byte so the string won't break
    char *temp = reallocate(ds->content, max(new_size, 1));
    if(temp == NULL){
        return DS_ERROR;
    }

    ds->content = temp;
    ds->end = temp + new_size;

    //Position still in range?
    if(position_offset < new_size){
        ds->position = temp + position_offset;
    }
    else {
        ds->position = ds->end - 1;
        *ds->position = '\0';    
    }

    return DS_SUCCESS;
}

//Allocate more bytes
int ds_reserve(Dynamic_String *ds, size_t amount)
{
    return ds_resize(ds, ds_capacity(ds) + amount);
}

//Deallocate part of memory
int ds_shrink(Dynamic_String *ds, size_t amount)
{
    if(amount >= ds_capacity(ds))
        return DS_ERROR;

    return ds_resize(ds, ds_capacity(ds) - amount);    
}

//Reduce allocated storage so it's just enough for the current content
int ds_shrink_to_fit(Dynamic_String *ds)
{
    return ds_resize(ds, ds_length(ds) + 1); //There's a '\0'    
}

//Push character to the end of string, return pointer to it
int ds_push_back(Dynamic_String *ds, int c)
{
    if(ds_is_full(ds) && expand(ds) == DS_ERROR){
        return DS_ERROR;
    }

    *ds->position++ = c;
    *ds->position = '\0';

    return DS_SUCCESS;        
}

//Append one dynamic string to another
int ds_append(Dynamic_String *destination, const Dynamic_String *source)
{
    size_t destination_space = ds_space(destination);
    size_t source_length = ds_length(source);

    //Check if there's space, try to allocate more if there isn't
    if(source_length > destination_space
    && expand_by_at_least(destination, source_length - destination_space)
    == DS_ERROR)
        return DS_ERROR;

    //Update info, where to append?
    char *insertion_point = destination->position;
    destination->position += source_length;
    *destination->position = '\0';

    memcpy(insertion_point, source->content, source_length);

    return DS_SUCCESS;
}

/* Append at most n characters from source to destination. Return location of 
source inside destination */ 
int ds_append_n(    Dynamic_String *destination,
                    const Dynamic_String *source, 
                    size_t max                        )
{
    //Avoid copying too much
    max = min(max, ds_length(source));

    size_t space = ds_space(destination);
    if(max > space && expand_by_at_least(destination, max - space) == DS_ERROR){
        return DS_ERROR;
    }

    char *insertion_point = destination->position;
    destination->position += max;
    *destination->position = '\0';

    memcpy(insertion_point, source->content, max);

    return DS_SUCCESS;
}

void ds_crop(Dynamic_String *ds, size_t total)
{

}

/* Compare two dynamic strings and return  0 if equal, positive if first
differing character is greater on str1 or negative if smaller */
int ds_compare(const Dynamic_String *str1, const Dynamic_String *str2)
{
    size_t length = min(ds_length(str1), ds_length(str2));
    return memcmp(str1->content, str2->content, length + 1);
}

//Compare up to n characters
int ds_compare_n(    const Dynamic_String *str1, 
                    const Dynamic_String *str2, 
                    size_t max                    )

{    
    size_t length = min(ds_length(str1), ds_length(str2));
    return memcmp(str1->content, str2->content, min(length + 1, max));
}

//Swap one dynamic string for another
void ds_swap(Dynamic_String *ds1, Dynamic_String *ds2)
{
    Dynamic_String temp = {ds1->content, ds1->position, ds1->end};

    ds1->content = ds2->content;
    ds1->position = ds2->position;
    ds1->end = ds2->end;

    ds2->content = temp.content;
    ds2->position = temp.position;
    ds2->end = temp.end;
}


////////////////////////////
/////////    Functions to work with dynamic and regular strings
///////////////////////////

/* Takes an already allocated regular string and put it into a container. Making
it a normal dynamic string. Container must not hold an allocated string or there
will be memory leaks. */
void ds_from_cstring(Dynamic_String *container, char *c_string)
{
    container->content = c_string;
    container->position = strchr(c_string, '\0');
    container->end = container->position + 1;
}

//Return an allocated copy of Dynamic_String content
char *ds_content_copy(const Dynamic_String *ds)
{    
    size_t length = ds_length(ds);

    char *temp = malloc(length + 1);
    if(temp == NULL)
        return NULL;

    return memcpy(temp, ds->content, length + 1);
}

//Reduce storage to the minimum, and return content as C string
char *ds_to_cstring(Dynamic_String *ds)
{
    //Assume reducing storage will work, if it doesn't return with extra space
    ds_shrink_to_fit(ds);
    return ds->content;
}

//Join a list of Dynamic_Strings and return their contents as C string
//The list must be delimited by NULL
char *ds_join_list_to_cstring(Dynamic_String **ds_list)
{
    //If all have length 0, return an string with '\0'
    size_t total_size = 1;
    for(size_t i = 0; ds_list[i] != NULL; ++i)
        total_size += ds_length(ds_list[i]);

    //Create a temporary string to hold all contents
    Dynamic_String temp;
    if(ds_allocate(&temp, total_size) == DS_ERROR)
        return NULL;

    for(size_t i = 0; ds_list[i] != NULL; ++i)
        ds_append(&temp, ds_list[i]);

    return temp.content;
}

//Append regular C string to Dynamic_String
int ds_append_cstring(Dynamic_String *dest, const char *c_string)
{
    return ds_append_cstring_by_length(dest, c_string, strlen(c_string));    
}

//Appending a C string of known length is faster
int ds_append_cstring_by_length(    Dynamic_String *dest,
                                    const char *c_string,
                                    size_t length                    )
{    
    size_t space = ds_space(dest);
    if(space < length && expand_by_at_least(dest, length - space) == DS_ERROR)
        return DS_ERROR;

    char *insertion_point = dest->position;
    dest->position += length;
    *dest->position = '\0';

    memcpy(insertion_point, c_string, length);

    return DS_SUCCESS;
}

Answer 1

This may be disappointing but: I didn't find much wrong with it. That said, there are a few small points that might be useful.

Reducing memory leaks

I almost didn't even write this one because the GTK library is notorious for leaking memory. With that said, there are a few things one can and should do. First, is that the top-level GtkWidget *window is created as a floating reference and isn't "owned" by anything. To make sure it's properly freed after it's done you can add this:

/* Main window */
GtkWidget *window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
g_object_ref_sink(window);  /* create a reference to the window */
/* ... */
gtk_main();
gtk_widget_destroy(window); /* ask for window to be released */
g_object_unref(window);     /* remove last reference to window */

There's a document that describes GTK memory management but consensus seems to be that if you use GTK your program WILL leak memory.

Use g_free for GTK allocated items

Within the spin function, the gtk_text_buffer_get_text function is called which allocates a new UTF-8 buffer. You're freeing it, which is good, but with free rather than g_free. It probably doesn't matter in this case, but get in the habit of using g_free for GTK+-allocated items.

Reconsider malformed strings

The program doesn't crash (good!) but it also doesn't like input of the form:

{big|fat} hog{

It prints to the console, but it could just as easily handle this input the same way that it handles

{big|fat} hog}

which is simply to print the trailing } like any other character.

Consolidate get_count and get_word_n

The content of the get_count and get_word_n functions is similar and overlapping and they are only called once back-to-back from within choose_one().

In all it seemed pretty solid code.

Answer 2

I just have some minor things in addition to what has been said:

• For organization, I would not mix up header files and libraries:

  #include <gtk/gtk.h>
  #include <stdlib.h>
  #include "dynamic_string.h"
  #include <time.h>
  

  You should have header files before libraries as this will avoid possible dependency issues resulting from forcing header files to be exposed to certain libraries.

  #include "dynamic_string.h"
  #include <gtk/gtk.h>
  #include <stdlib.h>
  #include <time.h>
  

• I don't think you need your own DS return codes when you already have <stdbool.h>:

  //Return codes
  #define DS_SUCCESS 1
  #define DS_ERROR 0
  

• Do not display an error message in get_word_n(); its intent does not include error-reporting. Instead, display it in the calling code if the function returns NULL. You could also add a comment next to the return statement, stating the reason(s) for returning NULL.

• To help with maintenance, consider using curly braces for single-line statements as well.