How do I copy a string to an array in C?

Question

I'm creating a simple stack with push and pop functions. I'm trying to push a series of strings into an array that's serving as the stack of memory. However, GDB keeps showing me that I didn't copy the strings correctly to the array. Anyone have ideas on how I can fix this?

/*************************************************************************
* stack.c
*
* Implements a simple stack structure for char* s.
************************************************************************/

// for strdup() in the testing code
#define _XOPEN_SOURCE 500

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

// the capacity of the stack
#define CAPACITY 10

//global variable used to keep track of pop and push

typedef struct
{
    // storage for the elements in the stack
    char* strings[CAPACITY];

    // the number of elements currently in the stack
    int size;
} stack;

// declare a stack (as a global variable)
stack s;

/**
 * Puts a new element into the stack onto the "top" of the data structure
 * so that it will be retrived prior to the elements already in the stack.
 */
bool push(char* str) 
{
    int i = 0;
    s.strings[i++] = strdup(str);
    ++s.size;
    return false;
}

/**
 * Retrieves ("pops") the last ("top") element off of the stack, following
 * the "last-in, first-out" (LIFO) ordering of the data structure. Reduces
 * the size of the stack.
 */
char* pop(void)
{
    int i = CAPACITY-1;
    return s.strings[i--];
}

/**
 * Implements some simple test code for our stack
 */
int main(void)
{
    // initialize the stack
    s.size = 0;

    printf("Pushing %d strings onto the stack...", CAPACITY);
    for (int i = 0; i < CAPACITY; i++)
    {
        char str[12];
        sprintf(str, "%d", i);
        push(strdup(str));
    }
    printf("done!\n");

    printf("Making sure that the stack size is indeed %d...", CAPACITY);
    assert(s.size == CAPACITY);
    printf("good!\n");

    printf("Making sure that push() now returns false...");
    assert(!push("too much!"));
    printf("good!\n");

    printf("Popping everything off of the stack...");
    char* str_array[CAPACITY];
    for (int i = 0; i < CAPACITY; i++)
    {
        str_array[i] = pop();
    }
    printf("done!\n");

    printf("Making sure that pop() returned values in LIFO order...");
    for (int i = 0; i < CAPACITY; i++)
    {
        char str[12];
        sprintf(str, "%d", CAPACITY - i - 1);
        assert(strcmp(str_array[i], str) == 0);
        free(str_array[i]);
    }
    printf("good!\n");

    printf("Making sure that the stack is now empty...");
    assert(s.size == 0);
    printf("good!\n");

    printf("Making sure that pop() now returns NULL...");
    assert(pop() == NULL);
    printf("good!\n");

    printf("\n********\nSuccess!\n********\n");

    return 0;
}

The two answers below are BOTH required for proper functioning :-) — anishsane
– anishsane, Commented Nov 25, 2013 at 8:36
Agreed, both need to change. However, the pop function is still giving me segmentation fault. Not sure why... — KishB87
– KishB87, Commented Nov 25, 2013 at 8:42

Michael · Accepted Answer · 2013-11-25 08:56:02Z

1

pop() functions always returns the same string:

char* pop(void)
{
    int i = CAPACITY-1; // i is the same!
    return s.strings[i--]; // the same pointer
}

I'd like to propose to change it in to the following:

char* pop(void)
{
    if (s.size == 0) return NULL;
    char *str =  s.strings[--s.size];
    s.strings[s.size] = NULL;
    return str;
}

It avoids you from storing an obsoleted pointer in a stack.

edited Nov 25, 2013 at 8:56

answered Nov 25, 2013 at 8:15

Michael

1,50314 silver badges26 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

KishB87 Over a year ago

Hmm, interesting point and thanks for that input. I didn't realize that. However, the program still isn't running to completion after inserting your suggestion. I think a problem is in the push function. I don't think the s.strings array is storing the strings properly.

Klas Lindbäck · Accepted Answer · 2013-11-25 08:56:45Z

1

Your push function always inserts strings in position 0.

Change like so:

bool push(char* str) 
{
    if (s.size == CAPACITY) return true;  // Stack is full!
    s.strings[s.size++] = strdup(str);
    return false;
}

And with pop (I stole this from Michaels answer and added the guard for empty stack):

char* pop(void)
{
    if (s.size == 0) return NULL;  // Stack is empty!
    char *str = s.strings[--s.size];
    s.strings[s.size] = NULL;
    return str;
}

edited Nov 25, 2013 at 8:56

answered Nov 25, 2013 at 8:28

Klas Lindbäck

33.4k5 gold badges61 silver badges85 bronze badges

7 Comments

KishB87 Over a year ago

That solved a big problem in the program. Now, the only issue is pop... The way it is right now, I'm getting segmentation fault... Don't know why.

Klas Lindbäck Over a year ago

See Michaels answer, but you also need to add a check to see if the stack is empty.

Michael Over a year ago

It is better to remove one "strdup" because calling it twice make a memory leak. For instance, it can be removed from push() or from push() call. Also I used my pop(), push() from this answer and fixed assert condition at line 78 - program runs fine.

KishB87 Over a year ago

Implemented Michael's solution. It decrements the size to zero so it solves that issue. However, I'm still getting segmentation faults with his implementation. GDB is telling me it's in line 53. Any idea what could be the problem?

Klas Lindbäck Over a year ago

The check for s.size==0 is necessary in pop. Without it, pop on an empty stack will read outside the allocated buffer causing undefined behaviour. (Apparantly seems to work in codepad but gives segmentation fault for KishB87)

|

Collectives™ on Stack Overflow

How do I copy a string to an array in C?

2 Answers 2

1 Comment

7 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

7 Comments

Your Answer

Sign up or log in

Post as a guest

Related