Попытка вернуть строку из очереди в C / free проблемы

Question

Я какое-то время работал над лабораторией для класса CSC, и, к сожалению, я немного устал от C (как вы, вероятно, заметите из кода).Я сталкиваюсь с двумя конкретными проблемами, обе из которых связаны с управлением памятью.

1) В операции удаления очереди я пытаюсь вернуть строковое значение из узла в конце очереди.Поскольку я также пытаюсь использовать free () и завершать работу этого узла после извлечения данных, мне нужно использовать метод, например strcpy (), для получения данных.Программа вызывает ошибки всякий раз, когда я пытаюсь использовать strcpy, и Valgrind заявляет, что недействительный r / w.

2) dequeue также неправильно обновляет структуру stringQueue по причинам, которые я не могу понять.У меня есть подобный код для стеков, где изменения сохраняются, но я могу продолжать работать в режиме dequeue весь день, и он фактически не удалит конечный узел.

Соответствующий код:

typedef struct node { 
  char data [strMax];
  struct node * next;
} queueNode;

typedef struct {
  queueNode * head;
  queueNode * tail;
} stringQueue;

char * dequeue(stringQueue *queue) {
  char * data = malloc(strMax * sizeof(char));
  if(empty(*queue)) {
    return "Null list!";
  }
  else if(!(queue->head)->next) { // One item in the queue.
    data = (queue->head)->data;
    //free(queue->head);
    queue->head = NULL;
    queue->tail = NULL;
  }
  else { // Multiple items in the queue.
    data = (queue->tail)->data;
    free(queue->tail);
    queueNode * trace = queue->head;
    while(trace->next) // Seek the last node in the queue.
      trace = trace->next;
    queue->tail = trace;
  }
  return data;
}

Answer 1

Ваша главная проблема в строках типа data = (queue->head)->data;. Вы не можете назначить массив, как это. Вы должны memcpy. (strcpy для строк с нулевым символом в конце, и я думаю, что это не так)

edit: вы также можете использовать strncpy, чтобы избежать переполнения буфера.

Answer 2

Я вижу несколько проблем с вашим кодом ...

Сначала вы не проверяете, что ваш аргумент queue не NULL.Тогда вы не включили определение empty(), но, вероятно, тестирование того, что queue-> head равно NULL, должно сказать, что список пуст.Здесь вы разыменовываете его перед тестированием, это действительный указатель, очень опасный.

Во-вторых, вы неверно используете некоторые данные, которые не используются должным образом.Когда вы делаете привязку data = (queue->head)->next;, вы теряете указатель на выделенную память, вы, вероятно, захотите сделать strncpy() здесь, например, strncpy(data, queue->head->data, strMax).После этого вы можете раскомментировать свой free().Функция, вызывающая вашу очередь один, должна будет free() эту строку позже, когда она больше не используется.Почему бы не выделить свой data только тогда, когда вы уверены, что список не пуст?Если вы не хотите этого делать, вам нужно free(), чтобы эта память не была заменена.

См. Код ниже.

queueNode* find_before_tail(stringQueue* queue)
{
   queueNode* node = NULL;

   if (!queue || !queue->head)
      return NULL;

   node = queue->head;
   while (node->next != queue->tail && node->next)
      node = node->next;

   return node;
}

char * dequeue(stringQueue *queue) {
  char *data = NULL;
  queueNode* to_queue = NULL;

  if(!queue || !queue->head) {
    /* Nothing to dequeue here... */
    return NULL;
  }

  data = malloc(strMax * sizeof(char));
  if (!data) {
    printf("Error with malloc()...\n");
    return NULL;
  }

  /* Only one element */
  if(!(queue->head)->next == queue->head) {
    strncpy(data, queue->head->data, strMax);

    free(queue->head);

    queue->head = NULL;
    queue->tail = NULL;
  }
  else {
    strncpy(data, queue->tail->data, strMax);

    to_dequeue = queue->tail;

    queue->head = queue->head->next;

    queue->tail = find_before_tail(queue);
    if (!queue->tail)
       return NULL;
    queue->tail->next = NULL;

    free(to_dequeue);
  }

  data[strMax - 1] = 0;

  return data;
}

Возможно, есть некоторые другие проблемы состальная часть вашего кода, судя по этому, но, надеюсь, это даст вам некоторую основу.

РЕДАКТИРОВАТЬ С ВАШИМ КОДОМ ОЧЕРЕДИ

Здесь вы опять не проверяете возвращаемое значениеmalloc().Вот версия с нециклическим связанным списком (я также обновил функцию dequeue() выше для работы с этим).

int enqueue(stringQueue *queue, char *item)
{
   queueNode * newNode = NULL;

   if (!queue || !item)
      return EINVAL;

   newNode = malloc(sizeof(queueNode));
   if (!newNode) {
      perror("malloc()");
      return errno;
   }

   strncpy(newNode->data, item, strMax);
   newNode->data[strMax - 1] = 0;

   if (!queue->head) {
       /* Element is queue and tail */
       queue->tail = newNode;
   }

   newNode->next = queue->head;
   queue->head = newNode;

   return 0; /* Everything was fine */
}

Я не тестировал код, но он должен быть очень похожк этому.В этом сценарии, когда у вас есть только один элемент, this_element->next равен NULL и не указывает на себя.

Answer 3

Возможно, вы сначала захотите объявить data как char * = NULL.Затем, когда вы хотите вернуть его, используйте data = asprintf("%s", (queue->tail)->data);.Это будет делать выделение и копирование строк только при необходимости и только требуемый размер.Тогда ваш вызывающий код должен взять на себя ответственность за освобождение самих этих данных.

В настоящее время у вас есть char[] в структуре вашего узла в памяти в куче.Позже вы устанавливаете указатель на член data структуры, а затем освобождаете структуру в памяти.У вас остается «висячий указатель», который указывает на то место, где раньше была структура.Попытка использовать этот указатель закончится почти неизбежным (или, что еще хуже, непредсказуемым поведением).