tags:

views:

38

answers:

1

so after debugging for awhile I've come to this in my program. It's supposed to copy name into a circular array for a queue. I am not exactly sure what I am doing wrong here and could really use some help.

qPtr->front is an int
qPtr->numElements is an int
qPtr->queueSize is an int
name is a string


strcpy(qPtr->name[(qPtr->front+qPtr->numElements)%qPtr->queueSize], name);

here is the queue function

int enqueue(struct line* qPtr, int items, int time, char name[50])
{
    int i;
    if (qPtr->numElements != qPtr->queueSize)
    {
        qPtr->numItems[(qPtr->front+qPtr->numElements)%qPtr->queueSize] = items;
        qPtr->timeEntered[(qPtr->front+qPtr->numElements)%qPtr->queueSize] = time;
        strcpy(qPtr->name[(qPtr->front+qPtr->numElements)%qPtr->queueSize], name);

        (qPtr->numElements)++;
    }
}

I've determined that it isn't when it dequeue's because if i run the program with commenting out the line i am having issue with it runs fine. When i run the program with

strcpy(qPtr->name[(qPtr->front+qPtr->numElements)%qPtr->queueSize], name);

commented out the program runs fine. Besides that what I expect it to do is to enqueue elements from a file so that they can be dequeued later but instead it crashes

+1  A: 

I have to ask: have you actually allocated space for the qptr->name[] variables. If your structure is simply something like:

struct line {
    int front;
    int numElements;
    int queueSize;
    char *name[100]; // or char **name where only first level is allocated.
}

then you will not have done so, which would lead to undefined behaviour when you try to strcpy the name into there.

Other than that, I would suggest the same thing I suggest to everyone submitting a problem.

  • What did you expect?
  • What actually happened?
  • Show us the code (you've already done this bit, I include it for completeness).

And, temporarily, insert some printf statements in that function so you can see the relevant fields before and after. That will usually lead you to see exactly what's going wrong.


Based on your comments, it appears you have an array of 50 character pointers in your structure, none of which has been allocated. What you were probably aiming for was an array of character pointers, each pointing to a 50-character string.

I would suggest having your structure containing:

char *name[MAX_SZ]; // or a variable length array (either c99 or malloc trickery).

and change:

strcpy(qPtr->name[(qPtr->front+qPtr->numElements)%qPtr->queueSize], name);

to:

qPtr->name[(qPtr->front+qPtr->numElements)%qPtr->queueSize] = strdup (name);

Just remember that, when you extract that from the list, you're responsible for freeing it when you're done.

If you're unlucky enough to be on a system that doesn't have strdup, well, here's one I prepared earlier :-)


For what it's worth, I dug up an old implementation from my code base, which you can use as you see fit:

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

// Would go in header file.

typedef enum {
    QERR_OKAY,
    QERR_NOMEM,
    QERR_INVALID,
    QERR_FULL
} eQueueErr;

typedef struct {
    size_t capacity;
    size_t used;
    size_t first;
    size_t next;
    char **list;
} tQueue;

const char *qErr (eQueueErr);
void qDump (char*,tQueue*);
tQueue *qCreate (size_t);
eQueueErr qPut (tQueue*,char*);
void qClear (tQueue*);
void qDestroy (tQueue*);

// Would go in C file.

// Returns textual representation of an error.

const char *qErr (eQueueErr e) {
    if (e == QERR_OKAY) return "Okay";
    if (e == QERR_NOMEM) return "NoMem";
    if (e == QERR_INVALID) return "Invalid";
    if (e == QERR_FULL) return "Full";
    return "?????";
}

// Dump function for debugging.

void qDump (char *desc, tQueue *q) {
    size_t i, j;
    printf ("%s: ", desc);
    printf ("capacity = %d, ", q->capacity);
    printf ("used = %d, ", q->used);
    printf ("first = %d, ", q->first);
    printf ("next = %d,", q->next);
    if (q->used > 0) {
        for (i = q->first, j = q->used; j > 0; i = (i + 1) % q->capacity, j--)
            printf (" %d=[%s]", i, q->list[i]);
    } else {
        printf (" no entries");
    }
    printf ("\n\n");
}

// Create a queue os a specified size.

tQueue *qCreate (size_t capacity) {
    tQueue *q;
    size_t i;

    // Need to create both the queue and the queue elements.

    if ((q = malloc (sizeof (tQueue))) == NULL)
        return NULL;
    if ((q->list = malloc (capacity * sizeof (char *))) == NULL) {
        free (q);
        return NULL;
    }

    // Set up accounting info and return it.

    q->capacity = capacity;
    q->used = q->first = q->next = 0;

    return q;
}

// Put something on the queue, return error if full, NULL or no memory available.

eQueueErr qPut (tQueue *q, char *s) {
    // Check if trying to put NULL or queue is full.

    if (s == NULL) return QERR_INVALID;
    if (q->used == q->capacity) return QERR_FULL;

    // Allocate new string and store if okay.

    q->list[q->next] = strdup (s);
    if (q->list[q->next] == NULL) return QERR_NOMEM;

    // Update accounting info and return success.

    q->next = (q->next + 1) % q->capacity;
    q->used++;

    return QERR_OKAY;
}

// Get an element from the queue (or NULL if empty).

char * qGet (tQueue *q) {
    char *s;

    // Just return NULL if empty.

    if (q->used == 0) return NULL;

    // Get it and adjust accounting info, then return it.

    s = q->list[q->first];
    q->first = (q->first + 1) % q->capacity;
    q->used--;

    return s;
}

// Clear a queue.

void qClear (tQueue *q) {
    size_t i, j;
    for (i = q->first, j = q->used; j > 0; i = (i + 1) % q->capacity, j--)
        free (q->list[i]);
    q->used = q->first = q->next = 0;
}

// Destroy a queue.

void qDestroy (tQueue *q) {
    qClear (q);
    free (q->list);
    free (q);
}

// Test program.

static char *nextLetter (void) {
    static char buffer[2];
    static char ch = 'A';
    buffer[0] = ch;
    buffer[1] = '\0';
    ch = (ch == 'Z') ? 'A' : (ch + 1);
    return buffer;
}

static void bigPut (tQueue *q, size_t num) {
    char *val;
    while (num-- > 0) {
        val = nextLetter();
        printf ("Putting '%s' returns: %s\n", val, qErr (qPut (q, val)));
        qDump ("After putting", q);
    }
}

static void bigGet (tQueue *q, size_t num) {
    char *val;
    while (num-- > 0) {
        val = qGet (q);
        printf ("Getting returns [%s]\n", val ? val : "<<null>");
        qDump ("After getting", q);
    }
}

int main (void) {
    tQueue *x;
    size_t i;
    char *val;

    if ((x = qCreate (5)) == NULL) return 1;
    qDump ("Create empty", x);

    printf ("Putting NULL returns: %s\n", qErr (qPut (x, NULL)));
    qDump ("After putting", x);

    bigPut (x, 6);
    bigGet (x, 4);
    bigPut (x, 2);
    bigGet (x, 4);
    bigPut (x, 3);

    qClear (x);
    qDump ("After clear", x);

    qDestroy (x);

    return 0;
}

Running this program generates the following debug output:

Create empty: capacity = 5, used = 0, first = 0, next = 0, no entries

Putting NULL returns: Invalid
After putting: capacity = 5, used = 0, first = 0, next = 0, no entries

Putting 'A' returns: Okay
After putting: capacity = 5, used = 1, first = 0, next = 1, 0=[A]

Putting 'B' returns: Okay
After putting: capacity = 5, used = 2, first = 0, next = 2, 0=[A] 1=[B]

Putting 'C' returns: Okay
After putting: capacity = 5, used = 3, first = 0, next = 3, 0=[A] 1=[B] 2=[C]

Putting 'D' returns: Okay
After putting: capacity = 5, used = 4, first = 0, next = 4, 0=[A] 1=[B] 2=[C] 3=[D]

Putting 'E' returns: Okay
After putting: capacity = 5, used = 5, first = 0, next = 0, 0=[A] 1=[B] 2=[C] 3=[D] 4=[E]

Putting 'F' returns: Full
After putting: capacity = 5, used = 5, first = 0, next = 0, 0=[A] 1=[B] 2=[C] 3=[D] 4=[E]

Getting returns [A]
After getting: capacity = 5, used = 4, first = 1, next = 0, 1=[B] 2=[C] 3=[D] 4=[E]

Getting returns [B]
After getting: capacity = 5, used = 3, first = 2, next = 0, 2=[C] 3=[D] 4=[E]

Getting returns [C]
After getting: capacity = 5, used = 2, first = 3, next = 0, 3=[D] 4=[E]

Getting returns [D]
After getting: capacity = 5, used = 1, first = 4, next = 0, 4=[E]

Putting 'G' returns: Okay
After putting: capacity = 5, used = 2, first = 4, next = 1, 4=[E] 0=[G]

Putting 'H' returns: Okay
After putting: capacity = 5, used = 3, first = 4, next = 2, 4=[E] 0=[G] 1=[H]

Getting returns [E]
After getting: capacity = 5, used = 2, first = 0, next = 2, 0=[G] 1=[H]

Getting returns [G]
After getting: capacity = 5, used = 1, first = 1, next = 2, 1=[H]

Getting returns [H]
After getting: capacity = 5, used = 0, first = 2, next = 2, no entries

Getting returns [<<null>]
After getting: capacity = 5, used = 0, first = 2, next = 2, no entries

Putting 'I' returns: Okay
After putting: capacity = 5, used = 1, first = 2, next = 3, 2=[I]

Putting 'J' returns: Okay
After putting: capacity = 5, used = 2, first = 2, next = 4, 2=[I] 3=[J]

Putting 'K' returns: Okay
After putting: capacity = 5, used = 3, first = 2, next = 0, 2=[I] 3=[J] 4=[K]

After clear: capacity = 5, used = 0, first = 0, next = 0, no entries
paxdiablo
yes I have done thatstruct line{ char* name[50]; int* timeEntered; int* numItems; int front; int numElements; int queueSize;};
Chris
@Chris, that gives you an array of 50 character _pointers_. What you need is a variable length array (depends on `queueSize` of character pointers, _each of which should be allocated to hold a 50-character string!_ In other words, it doesn't look like you've allocated space for the actual strings. It would probably be easier to use `strdup` if your system has it.
paxdiablo
wow thanks a lot. I was completely lost. I really appreciate it.
Chris