Как нажать и вытолкнуть пустой указатель в C - PullRequest
8 голосов
/ 23 марта 2019

У меня есть этот рабочий код:

#import <stdlib.h>
#import <stdio.h>

typedef struct myarray {
  int len;
  void* items[];
} MYARRAY;

MYARRAY *collection;

void
mypop(void** val) {
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len--];
}

void
mypush(void* val) {
  int len = collection->len++;
  collection->items[len] = val;
  puts(collection->items[len]);
}

int
main() {
  puts("Start");
  collection = malloc( sizeof *collection + (sizeof collection->items[0] * 1000) );
  collection->len = 0;
  puts("Defined collection");
  mypush("foo");
  puts("Pushed foo");
  mypush("bar");
  puts("Pushed bar");
  char str1;
  mypop((void*)&str1);
  puts("Popped bar");
  puts(&str1);
  char str2;
  mypop((void*)&str2);
  puts("Popped foo");
  puts(&str2);
  puts("Done");
  return 0;
}

Это выводит:

Start
Defined collection
foo
Pushed foo
bar
Pushed bar
(null)
Popped bar

bar
Popped foo
�ߍ
Done

Это должно вывести это вместо:

Start
Defined collection
foo
Pushed foo
bar
Pushed bar
bar
Popped bar
bar
foo
Popped foo
foo
Done

Будучи новичком в C, я не совсем уверен, что происходит или почему вывод «искажен» таким образом. Кажется, однако, что двойной указатель void** позволяет вам передать указатель и получить значение , не зная типа , так что да. Но интересно, можно ли показать, как этот код должен быть реализован, чтобы я мог понять, как это сделать.

Скомпилировано с лязгом:

clang -o example example.c

Обновление

Я обновил свой код, чтобы отразить последние ответы, но все еще не уверен, что malloc коллекции верен.

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

typedef struct myarray {
  int len;
  void* items[];
} MYARRAY;

MYARRAY *collection;

void
mypop(void** val) {
  --collection->len;
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len];
}

void
mypush(void* val) {
  int len = collection->len++;
  collection->items[len] = val;
  puts(collection->items[len]);
}

int
main() {
  puts("Start");
  collection = malloc( sizeof *collection + (sizeof collection->items[0] * 1000) );
  collection->len = 0;
  puts("Defined collection");
  mypush("foo");
  puts("Pushed foo");
  mypush("bar");
  puts("Pushed bar");
  char *str1;
  mypop((void**)&str1);
  puts("Popped bar");
  puts(str1);
  char *str2;
  mypop((void**)&str2);
  puts("Popped foo");
  puts(str2);
  free(collection);
  puts("Done");
  return 0;
}

Ответы [ 4 ]

6 голосов
/ 27 марта 2019

Есть несколько вещей, которые нужно исправить, но для новичка это неплохо.

  1. pop

Сначала нужно уменьшить значение len (ваш толчокделает правильно постинкремент).Это стек.

void mypop(void** val) {
     puts(collection->items[--collection->len]);
     *val = collection->items[collection->len];
}

Массивы начинаются с 0, поэтому

len = 0;
items[len++] = elem1;  // len is 0 for the assignment then incremented
items[len++] = elem2;  // len is 1 for the assignment then incremented

затем для получения значений

elem2 = items[--len];  // len is first decremented to 1
elem1 = items[--len];  // len is first decremented to 0
str

Вам нужен указатель на символы, a char *, для str1 и str2, поскольку pop() будет хранить указатель, а не один символ.

 char *str1;
 mypop((void **)&str1);
 puts("Popped bar");
 puts(str1);
 char *str2;
 mypop((void **)&str2);
 puts("Popped foo");
 puts(str2);
 puts("Done");
 return 0;

Это должно исправить видимое повреждение дисплея.Однако есть еще несколько интересных вещей

Распределение

Ваши программы работают, потому что ваше выделение велико, и items находится внутри struct, его пространство, вероятно, покрыто всем выделением.Но это делает предположение (вполне вероятное, чтобы быть справедливым), которое может привести к неопределенному поведению в некоторых ситуациях.

Но, чтобы быть чище, так как у вас есть две сущности, чтобы выделить, чтотребуется два выделения

collection = malloc( sizeof *collection );
collection->items = malloc( sizeof(collection->items[0]) * 1000 );

, чтобы впоследствии оба были освобождены.

В этом случае структура должна быть

typedef struct myarray {
  int len;
  void **;
} MYARRAY

Поскольку MYARRAY сам по себе довольно малВы также можете объявить это статически

static MYARRAY collection;
import

#import устарело, используйте вместо него #include.

5 голосов
/ 23 марта 2019

Одна проблема здесь:

void mypush(void* state) {
   DATA data = { state };
   int pos = collection.len++;
   collection.items[pos] = &data;
}

Обратите внимание, что в последней строке этой функции хранится указатель на локальную переменную data в вашем массиве items. Но как только возвращается функция mypush(), эта локальная переменная уничтожается, что означает, что указатель, сохраненный в массиве, больше не действителен! (теперь это висячий указатель ) Скорее всего, ваша ошибка сегментации возникает, когда вы позже пытаетесь прочитать этот недействительный указатель (который вызывает неопределенное поведение, и в этом случае сбой)

Чтобы избежать этого, просто сохраните переменную state напрямую, без привлечения локальной переменной data. Вы можете приводить другие типы указателей к (и из) void * по мере необходимости (при условии, что вы тщательно проверяете, чтобы ваши приведения соответствовали фактическому типу данных, на которые указывает указатель - с помощью void-pointers, компилятора не скажу, если вы применяете неподходящий тип!)

3 голосов
/ 27 марта 2019

Существуют две основные проблемы с вашим измененным кодом. Первый в функции mypop:

void
mypop(void** val) {
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len--];
}

Когда функция введена, в массиве collection->items всего collection->len, а индекс последнего равен collection->len - 1. Таким образом, collection->items[collection->len] читает элемент массива, в который еще не записано, и выделенная память имеет неопределенных значений до того, как оно будет записано. Поэтому, когда вы вызываете puts для этого значения, вы разыменовываете неверный указатель. Это вызывает неопределенное поведение . На вашей машине он печатает «(ноль)», но на моей он вылетает.

Это можно исправить, уменьшив сначала len:

void
mypop(void** val) {
  collection->len--;
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len];
}

Вторая проблема заключается в том, как вы сохраняете извлеченные значения:

  char str1;
  mypop((void*)&str1);
  puts("Popped bar");
  puts(&str1);
  char str2;
  mypop((void*)&str2);
  puts("Popped foo");
  puts(&str2);

Функция mypop ожидает void **, то есть адрес void *, но вы передаете адрес char. Когда mypop затем присваивает *val, он пытается записать sizeof(void *) байтов (скорее всего, 4 или 8 байтов), чтобы присвоить значение, но str1 и str2 имеют размер sizeof(char) == 1 байт. , Таким образом, это означает, что *val = ... записывает прошлое str1 и str2 в смежную память, которая ему не принадлежит. Это снова вызывает неопределенное поведение.

Поскольку char * - это то, что хранилось в вашем стеке, это должен быть адрес char *, который вы передаете mypop. Так что str1 и str2 указатели на char:

  char *str1;
  mypop((void**)&str1);
  puts("Popped bar");
  puts(str1);
  char *str2;
  mypop((void**)&str2);
  puts("Popped foo");
  puts(str2);

Это позволит правильно запустить вашу программу.

Кроме того, вы не освободили выделенную память, поэтому обязательно наберите free(collection) в конце вашей программы.

Вы также должны использовать #include вместо #import для включения заголовочных файлов, так как первый стандартизирован, а последний является расширением.

Относительно вашего malloc:

collection = malloc( sizeof *collection + (sizeof collection->items[0] * 1000) );

Это хорошо. Размер структуры с гибким членом массива не включает размер этого члена. Поэтому, когда пространство для такой структуры выделено, вам нужен размер структуры плюс размер для некоторого числа элементов массива. Это именно то, что вы сделали: выделенное пространство для структуры с гибким элементом массива, способным содержать 1000 элементов.

0 голосов
/ 29 марта 2019

Изменено несколько вещей, прокомментированных в коде ниже.

Вы должны заметить, что вы должны выделить одну collection структуру, у которой есть указатель на 1000 items, который тоже должен быть выделен, а затем освободить их. А в массивах C начинаются с 0, поэтому последний нажатый элемент - collection->items[collection->len - 1].

Я этого не делал, но одна общая практика при работе со строками C состоит в том, чтобы инициализировать все элементы в массиве равными нулю сразу после выделения, поэтому такие функции, как puts(), никогда не вызовут ошибку сегментации, потому что 0 интерпретируется как конец строки.

#include <stdio.h>

typedef struct myarray {
  int len;
  void** items;
} MYARRAY;

MYARRAY *collection;

void
mypop(void** val) {
  --collection->len;
  puts(collection->items[collection->len]);
  *val = collection->items[collection->len];
}

void
mypush(void* val) {
  collection->len++;
  collection->items[collection->len - 1] = val;  // 0-based index
  puts((char *)collection->items[collection->len - 1]); // must cast to char*
}

int
main() {
  puts("Start");
  collection = malloc(sizeof(MYARRAY)); // alloc one structure
  collection->items = malloc(sizeof(void *) * 1000);    // that have 1000 items
  collection->len = 0;
  puts("Defined collection");
  mypush("foo");
  puts("Pushed foo");
  mypush("bar");
  puts("Pushed bar");
  char *str1;
  mypop((void**)&str1);
  puts("Popped bar");
  puts(str1);
  char *str2;
  mypop((void**)&str2);
  puts("Popped foo");
  puts(str2);
  free(collection->items);  // need to deallocate this too
  free(collection);
  puts("Done");
  return 0;
}
...