Вы не допустили пустое значение, которое завершает строки, поэтому у вас классические переполнения буфера.
Вам также не нужно копировать имена три раза.Ваш текущий код:
int L1 = strlen( A->name );
int L2 = strlen( B->name );
int len = L1 + L2;
char *Ap = (char*)malloc(L1*sizeof(char)); strcpy(Ap,A->name);
char *Bp = (char*)malloc(L2*sizeof(char )); strcpy(Bp,B->name);
char *c = (char*)malloc(sizeof(char)*(len + 2));
c[0] = '('; strcat(c, Ap); strcat(c, Bp); c[len+1] = ')';
ret->name = (char*)malloc(sizeof(char)*(len + 2));
strcpy(ret->name, c);
ret->R = A->R; ret->C = B->C;
ret->dim = ret->R*ret->C;
free(Ap); free(Bp); free(c);
Это должно быть:
int L1 = strlen(A->name);
int L2 = strlen(B->name);
ret->name = (char *)malloc(L1 + L2 + sizeof("()")); // That adds 3
sprintf(ret->name, "(%s%s)", A->name, B->name);
ret->R = A->R;
ret->C = B->C;
ret->dim = ret->R * ret->C;
Это полностью исключает Ap, Bp и c и позволяет избежать проблем переполнения буфера.Я не уверен, что я бы хлопнул двумя именами вместе, как вы, но это ваш выбор.
Очевидно, это не является достаточной проблемой ... Есть и другие проблемы.
void clone_list( MatrixList *from, MatrixList *to ) {
if (from->head == NULL) {
to = NULL;
} else {
Node *tmp = from->head;
while( tmp != NULL ) {
Matrix *m_copy = (Matrix*)malloc(sizeof(Matrix*));
char *c_copy = (char*)malloc(sizeof(char*)*strlen(tmp->M->name));
strcpy(c_copy,tmp->M->name);
m_copy->name=c_copy;
m_copy->R=tmp->M->R;
m_copy->C=tmp->M->C;
m_copy->concat_matrices = concat_matrices;
to->append( to,m_copy );
tmp = tmp->next;
}
}
}
Первое назначение обнуляет локальный указатель;он ничего не делает с MatrixList, переданным в качестве цели.Предположительно это должно быть:
if (from->head == 0)
{
*to = *from;
}
Это делает оптовое копирование структуры, но устанавливает нулевой заголовок и хвост, и указатели на функции все в порядке - они могут использоваться совместно.Предполагая, что size в from было правильно равно нулю, оно будет правильным и в to.(Опять же, это, вероятно, не тот код, который вы используете.)
Следующая проблема связана с выделением памяти:
Matrix *m_copy = (Matrix*)malloc(sizeof(Matrix*));
Это выделяет память одного указателя, а не одну матрицу,Используйте любой из них:
Matrix *m_copy = (Matrix *)malloc(sizeof(*m_copy));
Matrix *m_copy = (Matrix *)malloc(sizeof(Matrix));
Это основной источник вашей проблемы (и тот, который valgrind найдет легко).
Когда+1 забывается один раз, он забывается много раз, но на этот раз он не вызывает проблем, если имя не является пустой строкой, потому что вы умножаетесь на 4 или 8 (32-битный или 64-битный) из-за sizeof(char *) вместо намеченного sizeof(char).
char *c_copy = (char*)malloc(sizeof(char*)*strlen(tmp->M->name));
strcpy(c_copy,tmp->M->name);
Вероятно, это должно быть:
m_copy->name = (char *)malloc(strlen(tmp->M->name) + 1);
strcpy(m_copy->name, tmp->M->name);
Я бы, вероятно, использовал имя типа old вместо tmp.Я также упускаю из виду, что ранее не напоминал, что вам следует тщательно проверять каждый возврат при каждом выделении памяти.Или используйте набор функций покрытия для подпрограмм выделения памяти, которые выполняют проверку для вас (часто называемый xmalloc() или emalloc() и т. Д.).
Приведенный ниже код, похоже, не копирует *Член 1056 *, который является ошибкой, если вы когда-либо зависите от него.
Я не совсем рад, что вы, похоже, полагаетесь на список to, который соответствующим образом инициализируется перед вызовом clone_list().В частности, члены head, tail и size здесь не обнуляются, а указатели функций не устанавливаются.Я был бы счастлив увидеть что-то вроде:
*to = *from; // Copy function pointers
to->head = 0;
to->tail = 0;
to->size = 0;
Node *old = from->head;
for (Node *old = from->head; old != NULL; old = old->next)
{
Matrix *m_copy = clone_matrix(old->M);
to->append(to, m_copy);
}
Или даже:
matrixlist_initialize(to);
Node *old = from->head;
for (Node *old = from->head; old != NULL; old = old->next)
{
Matrix *m_copy = clone_matrix(old->M);
to->append(to, m_copy);
}
Функция clone_matrix() выглядит следующим образом:
Matrix *clone_matrix(const Matrix *old)
{
Matrix *m_copy = (Matrix*)malloc(sizeof(*m_copy));
m_copy->name = (char*)malloc(strlen(old->name)+1);
strcpy(m_copy->name, old->name);
m_copy->R = old->R;
m_copy->C = old->C;
m_copy->dim = old->dim;
m_copy->concat_matrices = concat_matrices;
return(m_copy);
}
Я скачал версию вашего кода, и теперь она работает более или менее.Вы должны компилировать как минимум с -Wall в качестве опции компилятора;Я отказываюсь компилировать с чем-то меньшим и обычно тоже использую -Wextra.Я делаю слишком много простых ошибок, чтобы не использовать компилятор в полной мере, и пока вы учитесь, вы тоже.(Я программировал на C чуть более четверти века; компилятор все еще ловит опечатки и другие глупые ошибки, но у меня редко бывают большие проблемы после компиляции кода.)
Когда я включаю -Wall, была проблема с (неиспользованной) функцией perm(), потому что она не возвращает значение, даже если она говорит, что это будет, и была проблема, потому что правильное определение для main() с аргументами - int main(int argc, char **argv)и ты скучал по одной из звезд.Кроме этого, похоже, что теперь он работает нормально - вы можете продолжить разработку.
В POSIX есть функция с именем strdup(), которая надежно дублирует строку.Это полезно и позволяет избежать ошибок, связанных с ошибками.
Вам следует рассмотреть использование заголовков .Они для объявлений, в первую очередь.Если вы явно используете inline (ваш код еще нет), может быть целесообразно включить inline функции в заголовок, но в противном случае тела функций не должны быть в заголовках.Они должны быть в исходных файлах (суффикс .c).Каждый заголовок должен содержать минимально необходимую информацию для кода, который использует функциональные возможности, предоставляемые источником для использования.Он не должен включать посторонние заголовки, но он должен включать все необходимые заголовки.В matrix.h вы включаете <stdio.h>, который не нужен.И если вы удалите код, ни <string.h>, ни <stdlib.h> также не понадобятся.