Ошибка сегментации с помощью strcat - PullRequest
2 голосов
/ 16 июля 2010

У меня небольшая проблема с ошибками strcat и сегментации.Ошибка выглядит следующим образом:

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x0000000000000000
0x00007fff82049f1f in __strcat_chk ()
(gdb) where
#0  0x00007fff82049f1f in __strcat_chk ()
#1  0x0000000100000adf in bloom_operation (bloom=0x100100080, item=0x100000e11 "hello world", operation=1) at bloom_filter.c:81
#2  0x0000000100000c0e in bloom_insert (bloom=0x100100080, to_insert=0x100000e11 "hello world") at bloom_filter.c:99
#3  0x0000000100000ce5 in main () at test.c:6

bloom_operation выглядит следующим образом:

int bloom_operation(bloom_filter_t *bloom, const char *item, int operation)
{
    int i;    

    for(i = 0; i < bloom->number_of_hash_salts; i++)
    {
        char temp[sizeof(item) + sizeof(bloom->hash_salts[i]) + 2];
        strcat(temp, item);
        strcat(temp, *bloom->hash_salts[i]);

        switch(operation)
        {
            case BLOOM_INSERT:
                bloom->data[hash(temp) % bloom->buckets] = 1;
                break;
            case BLOOM_EXISTS:
                if(!bloom->data[hash(temp) % bloom->buckets]) return 0;
                break;
        }    
    }

    return 1;
}

Строка с ошибкой - это строка second strcat.Блум-> hash_salts являются частью структуры, определенной следующим образом:

typedef unsigned const char *hash_function_salt[33];
typedef struct {
    size_t buckets;    
    size_t number_of_hash_salts;
    int bytes_per_bucket;
    unsigned char *data;
    hash_function_salt *hash_salts;
} bloom_filter_t;

И здесь они инициализируются:

bloom_filter_t* bloom_filter_create(size_t buckets, size_t number_of_hash_salts, ...) 
{
    bloom_filter_t *bloom;
    va_list args;
    int i;

    bloom = malloc(sizeof(bloom_filter_t));
    if(bloom == NULL) return NULL;

    // left out stuff here for brevity...

    bloom->hash_salts = calloc(bloom->number_of_hash_salts, sizeof(hash_function_salt));

    va_start(args, number_of_hash_salts);

    for(i = 0; i < number_of_hash_salts; ++i)
        bloom->hash_salts[i] = va_arg(args, hash_function_salt);

    va_end(args);

    // and here...
}

И bloom_filter_create вызывается следующим образом:

bloom_filter_create(100, 4, "3301cd0e145c34280951594b05a7f899", "0e7b1b108b3290906660cbcd0a3b3880", "8ad8664f1bb5d88711fd53471839d041", "7af95d27363c1b3bc8c4ccc5fcd20f32");

Я делаю что-то не так, но я действительно потерян, что ли.Заранее спасибо,

Бен.

Ответы [ 5 ]

9 голосов
/ 16 июля 2010

Я вижу пару проблем:

char temp[sizeof(item) + sizeof(bloom->hash_salts[i]) + 2];

sizeof(item) вернет только 4 (или 8 на 64-битной платформе).Вы, вероятно, должны использовать strlen () для фактической длины.Хотя я не думаю, что вы можете объявить это в стеке таким образом с помощью strlen (хотя я думаю, что, может быть, я видел, что кто-то указал, что это возможно с более новыми версиями gcc - я могу пообедать с этим).

Другая проблема в том, что временный массив не инициализирован.Так что первый strcat может не записывать в начало массива.Ему нужно, чтобы NULL (0) вставлялся в первый элемент перед вызовом strcat.

Возможно, он уже содержится в отбитом коде, но я не видел, чтобы вы инициализировали элемент number_of_hash_saltsв структуре.

4 голосов
/ 16 июля 2010

Вам нужно использовать strlen, а не sizeof.item передается как указатель, а не как массив.

Строка:

char temp[sizeof(item) + sizeof(bloom->hash_salts[i]) + 2];

сделает temp 34x длиной указателя + 2. Размер элементаразмер указателя, а sizeof(bloom->hash_salts[i]) в настоящее время в 33 раза больше размера указателя.

Вам необходимо использовать strlen для item, чтобы вы знали фактическое количество символов.

Во-вторых, bloom->hash_salts[i] - это hash_function_salt, который представляет собой массив из 33 указателей на символ.Кажется, что hash_function_salt должен быть определен как:

, так как вы хотите, чтобы он содержал 33 символа, а не 33 указателя.Также следует помнить, что когда вы передаете строковый литерал в bloom_filter_create, вы передаете указатель.Это означает, что для инициализации массива hash_function_salt мы используем memcpy или strcpy.memcpy быстрее, когда мы знаем точную длину (как здесь):

Таким образом, мы получаем:

typedef unsigned char hash_function_salt[33];

и bloom_filter_create:

memcpy(bloom->hash_salts[i], va_arg(args, char*), sizeof(bloom->hash_salts[i]));

Возвращениедля bloom_operation мы получаем:

char temp[strlen(item) + sizeof(bloom->hash_salts[i])];
strcpy(temp, item);
strcat(temp, bloom->hash_salts[i]);

Мы используем strlen для элемента, так как это указатель, но sizeof для hash_function_salt, который является массивом фиксированного размера char.Нам не нужно ничего добавлять, потому что hash_function_salt уже содержит место для NUL.Сначала используем strcpy.strcat для случая, когда у вас уже есть строка с NUL-окончанием (чего у нас нет).Обратите внимание, что мы опускаем *.Это была ошибка из-за неверного определения типа.

2 голосов
/ 16 июля 2010

Во-первых, как все говорили, вы измерили temp на основе размеров двух указателей, а не длины строк. Теперь вы исправили это и сообщили, что симптом перешел на вызов strlen().

Это показывает более тонкую ошибку.

Вы инициализировали массив bloom->hash_salts[] из указателей, возвращаемых va_arg(). Эти указатели будут иметь ограниченный срок службы. Они могут даже не пережить вызов va_end(), но они почти наверняка не переживут вызов bloom_filter_create(). Позже, в bloom_filter_operation(), они указывают на произвольные места, и вы обречены на какой-то интересный провал.

Редактировать: Для решения этой проблемы требуется, чтобы указатели, хранящиеся в массиве hash_salts, имели достаточный срок жизни. Один из способов справиться с этим - выделить для них хранилище, скопировав их из массива varargs, например:

// fragment from bloom_filter_create()
bloom->hash_salts = calloc(bloom->number_of_hash_salts, sizeof(hash_function_salt));
va_start(args, number_of_hash_salts);
for(i = 0; i < number_of_hash_salts; ++i)
        bloom->hash_salts[i] = strdup(va_arg(args, hash_function_salt));
va_end(args);

Позже вам нужно будет перебрать hash_salts и вызвать free() для каждого элемента, прежде чем освободить сам массив указателей.

Другим подходом, который потребует больших затрат на инициализацию, но меньше усилий для освобождения, будет выделение массива указателей вместе с достаточным пространством для всех строк в одном выделении. Затем скопируйте строки и заполните указатели. Много кода, чтобы получить право на очень маленькое преимущество.

2 голосов
/ 16 июля 2010

При расчете размера массива для temp используется sizeof(bloom->hash_salts[i]) (это просто размер указателя ), но затем вы разыменовываете указатель и пытаетесь скопировать всю строку в temp.

1 голос
/ 16 июля 2010

Вы уверены, что тип hash_function_salt определен правильно?У вас может быть слишком много * *:

(gdb) ptype bloom
type = struct {
    size_t buckets;
    size_t number_of_hash_salts;
    int bytes_per_bucket;
    unsigned char *data;
    hash_function_salt *hash_salts;
} *
(gdb) ptype bloom->hash_salts
type = const unsigned char **)[33]
(gdb) ptype bloom->hash_salts[0]
type = const unsigned char *[33]
(gdb) ptype *bloom->hash_salts[0]
type = const unsigned char *
(gdb) 
...