4
\$\begingroup\$

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;
}
\$\endgroup\$
  • \$\begingroup\$ Tried to compile but apparently you've made some changes to the interface of Dynamic_String. Specifically, is there a new member cstring? \$\endgroup\$ – Edward May 5 '14 at 18:48
  • \$\begingroup\$ @Edward it's just a different name for content. Sorry about that. \$\endgroup\$ – 2013Asker May 5 '14 at 19:08
  • \$\begingroup\$ @Edward I tried to compile too and I noticed there are many changes. Please check the code I just posted. I hadn't posted it before because there are a lot of improvements that could be made. \$\endgroup\$ – 2013Asker May 5 '14 at 19:17
6
\$\begingroup\$

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.

\$\endgroup\$
5
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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