0

I'm writing a small program that takes a set of mathematical expressions (x^2, sin x, etc.) from a text file and plots them onto a graph, when reading each expression they are stored into a struct declared as follows:

typedef struct Expression
{
    int r, g, b;
    char* expression;
} Expression;

Note: the r, g, and b variables are the colour of the line that will be plotted, the expression is stored as a string.

My linked list and node are declared as:

typedef struct LinkedListNode
{
    struct Expression *expression;
    struct LinkedListNode *next;
} LinkedListNode;

typedef struct LinkedList
{
    LinkedListNode *head;
} LinkedList;

All I do to read the expressions in is run a while loop until EOF (I know all this works fine):

Expression* expression = (Expression*)malloc(sizeof(Expression));
LinkedList* expressionList;
expressionList = createEmptyList();

while(!feof(readFile))
{
    fscanf(readFile, "%d %d %d %[^\n]s", &r, &g, &b, expr);
    fgetc(readFile);

    expression -> r = r;
    expression -> g = g;
    expression -> b = b;
    expression -> expression = expr;

    insertNodeFront(expressionList, expression);
}

Now, the insertNodeFront function is where I'm having problems:

void insertNodeFront(LinkedList *inList, Expression *inExpression)
{
    LinkedListNode *newNode;

    newNode = (LinkedListNode*)malloc(sizeof(LinkedListNode));
    newNode -> expression = inExpression;
    newNode -> next = inList -> head;
    inList -> head = newNode;
}

I think it's this line in particular where the issue lies:

newNode -> expression = inExpression;

Because each expression member of every linked list is still pointing to the original Expression struct they will all end up with the same value in the end (obviously problematic), so I assume I have to create a duplicate of the inExpression pointer within insertNodeFront but I'm not exactly sure how to do this, all my attempts at using memcpy have resulted in crashes (although this is my first time using memcpy so I'm probably doing something wrong).

Any advice would be helpful, Thanks

Magikjak
  • 1
  • 1
  • [`while (!feof(f))` is always wrong.](https://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) – The Paramagnetic Croissant Oct 24 '14 at 20:45
  • I haven't been having any issues with it, what else should I use instead? while(fgetc(f) != EOF)? – Magikjak Oct 24 '14 at 20:57
  • @user2978566 "I haven't been having any issues with it" - that would be because you never *check* to see if your `fscanf` and `fgetc` *succeed* before marching on under the assumption they did. The [**link**](http://stackoverflow.com/a/5432517/1322972) provided in the comment explains why it is almost-always wrong. – WhozCraig Oct 24 '14 at 21:02

1 Answers1

1

You cannot assign newNode -> expression = inExpression; since the pointer inExpression is not preserved in the program. You need to allocate storage for newNode -> expression and save a copy of the string in inExpression. The simple way to do this is to include string.h and then use strdup before calling insertNodeFront:

expression -> expression = strdup (expr);

You have then created permanent storage for the string. Note: you are responsible for freeing the memory allocated by strdup.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
  • In such cases I've always wondered if implementing a reference counted string structure could be worth the effort. But I guess ultimately "it depends". – Jongware Oct 24 '14 at 21:08