Re: Где ошибка? - C/C++

Есть структура:

struct _xStringList
{    
    char ***list;

    int size;

    /*<private>*/
    int al;
};

В ней хранится список строк.
Есть 3 функции для создания нового списка, добавления строк и удаления всего списка.
При запуске программы, где используются данные функции вне VS всё нормально, при запуске в VS
появляется ошибка в *_x_string_list_free. Что может быть за причина?

xStringList *_x_string_list_new(void)
{
    xStringList *lst = (xStringList *)x_malloc(sizeof(xStringList));
    lst->list = (char ***)x_malloc(sizeof(char **) * 255);
    lst->al = 255;
    lst->size = 0;
    return lst;
}
void x_string_list_add(xStringList *lst, int count, ...)
{
    char ***tmp;
    int i;
P1:
    if (lst != NULL)
    {
P2:
        
        if (lst->al > lst->size)
        {
            int i;
            va_list par; 
            lst->list[lst->size] = (char **)x_malloc(sizeof(char *) * count);
            va_start(par, count);
            for (i = 0; i < count; i++)
            {
                lst->list[lst->size][i] = x_strdup(va_arg(par, char *));
            }
            lst->list[lst->size++][i] = NULL;
            va_end(par);
        }
        else
        {    
            tmp = lst->list;
            lst->list = (char ***)x_malloc(sizeof(char **)*(lst->al*=2));
            for (i = 0; i < lst->size; i++) lst->list[i] = tmp[i];
            x_free(tmp);
            goto P2;
        }
    }
    else
    {
        lst = x_string_list_new();    
        goto P1;
    }
}

xStringList *_x_string_list_free(xStringList *lst)
{
    int i, j;
    if (lst != NULL)
    {
        for (i = 0; i < lst->size; i++)
        {
            for (j = 0; lst->list[i][j] != NULL; j++)
            {
                x_free(lst->list[i][j]);
            }    
            x_free(lst->list[i]); /* Здесь ошибка!
        } 
        x_free(lst->list);
        x_free(lst);
    }
    return NULL;
}

Вот правильное решение:

#include <list>

std::list<...> lst;

А твой код вызывает ужас

. Прости.

Здравствуйте, Vamp, Вы писали:

V>Вот правильное решение:

V>

V>#include <list>

V>std::list<...> lst;
V>

Нужен чистый С!
V>А твой код вызывает ужас

. Прости.
Я знаю

void x_string_list_add(xStringList *lst, int count, ...)
{
/* ... */        
        if (lst->al > lst->size)
        {
            int i;
            va_list par; 
            lst->list[lst->size] = (char **)x_malloc(sizeof(char *) * count);
            va_start(par, count);
            for (i = 0; i < count; i++)
            {
                lst->list[lst->size][i] = x_strdup(va_arg(par, char *));
            }
/* Зачем нужна вот эта строчка ?  VVV           */
            lst->list[lst->size++][i] = NULL;
/* Возможно, ей затирается служебная инф-я хипа */
/* И в ф-ии free все падает                     */
            va_end(par);
        }
/* ... */        
}

Здравствуйте, Аноним, Вы писали:

А>Есть структура:

А>[ccode]
А>struct _xStringList
А>{
А> char ***list;

Вот здесь. Три звездочки — слишком большая крепость

. Сделай typedef, чтобы по крайней мере до двух сократить.

В C не нужно явно приводить void * к типу твоего указателя. Так что можно писать прямо:
xStringList *lst = x_malloc(sizeof(xStringList)); Я бы использовал calloc, malloc не обнуляет память. Да и кто за тебя будет на NULL проверять? В x_string_list_add нужно циклы явные сделать а не с помощью goto.

Понизь сложность кода и ошибка станет видна.

У него тут даже не malloc а какой-то x_alloc

И вообще, мне кажется, что программы на "чистом С" вполне могут выглядеть проще, чище и элегантней без всяких кошмариков.

Здравствуйте, dik o-braz, Вы писали:

DOB>

DOB>void x_string_list_add(xStringList *lst, int count, ...)
DOB>{
DOB>/* ... */        
DOB>        if (lst->al > lst->size)
DOB>        {
DOB>            int i;
DOB>            va_list par; 
            lst->>list[lst->size] = (char **)x_malloc(sizeof(char *) * count);
DOB>            va_start(par, count);
DOB>            for (i = 0; i < count; i++)
DOB>            {
                lst->>list[lst->size][i] = x_strdup(va_arg(par, char *));
DOB>            }
DOB>/* Зачем нужна вот эта строчка ?  VVV           */
            lst->>list[lst->size++][i] = NULL;
DOB>/* Возможно, ей затирается служебная инф-я хипа */
DOB>/* И в ф-ии free все падает                     */
DOB>            va_end(par);
DOB>        }
DOB>/* ... */        
DOB>}

DOB>

Именно. Я уже нашёл, но всё равно спасибо. Просто память выделяю для count элементов:

 lst->list[lst->size++][i] = NULL;

, а инициализирую count+1 элемент.
Надо писать :

lst->list[lst->size] = (char **)x_malloc(sizeof(char *) * (count+1));

Тема закрыта.

Здравствуйте, Lorenzo_LAMAS, Вы писали:

L_L>У него тут даже не malloc а какой-то x_alloc

L_L>И вообще, мне кажется, что программы на "чистом С" вполне могут выглядеть проще, чище и элегантней без всяких кошмариков.
x_malloc : тот же malloc, но с проверкой на ошибки выделения памяти.
x_free : тот же free, но с проверкой на равенство NULL передаваемого указателя и с присваиванием этому указателю NULL :

#define __STANDART_FREE__(point) free((void *)(point))
#define __FREE__(point) ((point) = __mem_free__((point)))
#define x_free(point) __FREE__((void *)(point))

void *__mem_free__(void *point)
{
    assert(point != NULL);
    __STANDART_FREE__(point);
    return NULL;
}

Ну что же...
Даст ист рефакторинг!
Я постарался максимально сохранить логику автора, только сделал программу чуть более читаемой и избавился от готу.

#include <malloc.h>
#include <string.h>
#include <sys/varargs.h>

typedef char* str;
typedef str* strlst;


typedef struct _xStringList {
    strlst* list;
    int size;

    /*<private>*/
    int al;
} xStringList;

void x_string_list_add(xStringList* lst, int count, ...);

xStringList* x_string_list_new()
{
    xStringList *lst = (xStringList *)x_malloc(sizeof(xStringList)); //allocator's left as is
    if (!lst)
        return NULL;
    lst->list = (strlst*) x_malloc(sizeof(strlst) * 255);
    if (!lst->list) {
        free(lst);
        return NULL;
    }
    lst->al = 255;
    lst->size = 0;
    return lst;
}


void x_string_list_add(xStringList* lst, int count, ...)
{
    strlst* tmp;
    va_list par;
    int i;
    
    if (!lst) 
    lst=x_string_list_new();
    if (!lst)
    return;
    //logic left as is. Although you should mention, that such allocated list would be lost after
    //function returns.        
    
    if (lst->al <= lst->size) {
    //again logic left as is. But we could consider using some form of x_realloc?
    tmp = lst->list;
    lst->al*=2;
    lst->list = (strlst*)x_malloc(sizeof(strlst)*(lst->al));
    if (!lst->list)
        return;
        for (i = 0; i < lst->size; ++i) 
           lst->list[i] = tmp[i];
        x_free(tmp);
    }
    
    lst->list[lst->size] = (strlst)x_malloc(sizeof(str) * (count+1));
    va_start(par, count);
    for (i = 0; i < count; i++)
         lst->list[lst->size][i] = x_strdup(va_arg(par, str)); 

    lst->list[lst->size][i]=NULL;
    ++lst->size;
    va_end(par);
}

void x_string_list_free(xStringList *lst)
{
    int i, j;
    if (!lst)
        return;
    for (i = 0; i < lst->size; i++) {
        for (j = 0; lst->list[i][j]; ++j)
             x_free(lst->list[i][j]);
         x_free(lst->list[i]);
    } 
  x_free(lst->list);
  x_free(lst);
}

int main() {
    xStringList* thelist=x_string_list_new();
    x_string_list_add(thelist, 2, "one string", "another string");
    x_string_list_free(thelist);
    return 0;
}

Здравствуйте, Аноним, Вы писали:

А>Здравствуйте, Lorenzo_LAMAS, Вы писали:

L_L>>У него тут даже не malloc а какой-то x_alloc

L_L>>И вообще, мне кажется, что программы на "чистом С" вполне могут выглядеть проще, чище и элегантней без всяких кошмариков.
А>x_malloc : тот же malloc, но с проверкой на ошибки выделения памяти.
А>x_free : тот же free, но с проверкой на равенство NULL передаваемого указателя и с присваиванием этому указателю NULL :

А>

А>#define __STANDART_FREE__(point) free((void *)(point))
А>#define __FREE__(point) ((point) = __mem_free__((point)))
А>#define x_free(point) __FREE__((void *)(point))

А>void *__mem_free__(void *point)
А>{
А>    assert(point != NULL);
А>    __STANDART_FREE__(point);
А>    return NULL;
А>}
А>

assert работает только в debug версии программы, а что на счет release-а?

Здравствуйте, Анатолий Широков, Вы писали:

АШ>Здравствуйте, Аноним, Вы писали:

А>>Здравствуйте, Lorenzo_LAMAS, Вы писали:

L_L>>>У него тут даже не malloc а какой-то x_alloc

L_L>>>И вообще, мне кажется, что программы на "чистом С" вполне могут выглядеть проще, чище и элегантней без всяких кошмариков.
А>>x_malloc : тот же malloc, но с проверкой на ошибки выделения памяти.
А>>x_free : тот же free, но с проверкой на равенство NULL передаваемого указателя и с присваиванием этому указателю NULL :

А>>

А>>#define __STANDART_FREE__(point) free((void *)(point))
А>>#define __FREE__(point) ((point) = __mem_free__((point)))
А>>#define x_free(point) __FREE__((void *)(point))

А>>void *__mem_free__(void *point)
А>>{
А>>    assert(point != NULL);
А>>    __STANDART_FREE__(point);
А>>    return NULL;
А>>}
А>>

АШ>assert работает только в debug версии программы, а что на счет release-а?
А она и нужна только на время отладки программы. В крайнем случае можно определить

#define _DEBUG
код
#undef _DEBUG

А для случаев, когда неизвестно, равен ли указатель NULL по логике программы, есть easy версия функции:

void *__mem_free_easy__(void *point)
{
        if (point) __STANDART_FREE__(point);
        return NULL;
}

Здравствуйте, Vamp, Вы писали:

V>Ну что же...
V>Даст ист рефакторинг!
V>Я постарался максимально сохранить логику автора, только сделал программу чуть более читаемой и избавился от готу.

V>

V>#include <malloc.h>
V>#include <string.h>
V>#include <sys/varargs.h>

V>typedef char* str;
V>typedef str* strlst;


V>typedef struct _xStringList {
V>    strlst* list;
V>    int size;

V>    /*<private>*/
V>    int al;
V>} xStringList;

V>void x_string_list_add(xStringList* lst, int count, ...);

V>xStringList* x_string_list_new()
V>{
V>    xStringList *lst = (xStringList *)x_malloc(sizeof(xStringList)); //allocator's left as is
V>    if (!lst)
V>        return NULL;
    lst->>list = (strlst*) x_malloc(sizeof(strlst) * 255);
V>    if (!lst->list) {
V>        free(lst);
V>        return NULL;
V>    }
    lst->>al = 255;
    lst->>size = 0;
V>    return lst;
V>}


V>void x_string_list_add(xStringList* lst, int count, ...)
V>{
V>    strlst* tmp;
V>    va_list par;
V>    int i;
    
V>    if (!lst) 
V>    lst=x_string_list_new();
V>    if (!lst)
V>    return;
V>    //logic left as is. Although you should mention, that such allocated list would be lost after
V>    //function returns.        
    
V>    if (lst->al <= lst->size) {
V>    //again logic left as is. But we could consider using some form of x_realloc?
V>    tmp = lst->list;
    lst->>al*=2;
    lst->>list = (strlst*)x_malloc(sizeof(strlst)*(lst->al));
V>    if (!lst->list)
V>        return;
V>        for (i = 0; i < lst->size; ++i) 
           lst->>list[i] = tmp[i];
V>        x_free(tmp);
V>    }
    
    lst->>list[lst->size] = (strlst)x_malloc(sizeof(str) * (count+1));
V>    va_start(par, count);
V>    for (i = 0; i < count; i++)
         lst->>list[lst->size][i] = x_strdup(va_arg(par, str)); 

    lst->>list[lst->size][i]=NULL;
V>    ++lst->size;
V>    va_end(par);
V>}

V>void x_string_list_free(xStringList *lst)
V>{
V>    int i, j;
V>    if (!lst)
V>        return;
V>    for (i = 0; i < lst->size; i++) {
V>        for (j = 0; lst->list[i][j]; ++j)
V>             x_free(lst->list[i][j]);
V>         x_free(lst->list[i]);
V>    } 
V>  x_free(lst->list);
V>  x_free(lst);
V>}

V>int main() {
V>    xStringList* thelist=x_string_list_new();
V>    x_string_list_add(thelist, 2, "one string", "another string");
V>    x_string_list_free(thelist);
V>    return 0;
V>}
V>

Класс! Именно так написал бы "Я" сегодняшний. Всё это дело было написано 7 лет назад, и только для фиксированного числа параметров. Сегодня понадобилось вновь, теперь для переменного.

#define START_CAPACITY (int)16

typedef char* String;
typedef String* StringList;

typedef struct StringListTag
{
    int size;
    int capacity;
    StringList list[1]; /* важно чтобы это был последний член структуры */ 
} xStringList; 

xStringList* x_string_list_new(int capacity)
{
    xStringList *lst = (xStringList *)malloc(sizeof(xStringList)+sizeof(StringList)*((capacity ? capacity : START_CAPACITY)-1));

    lst->size = 0;
    lst->capacity = capacity ? capacity : START_CAPACITY;
    memset(lst->list, 0, sizeof(StringList)*capacity);
    
    return lst;
}

extern void x_string_list_free(xStringList *);

xStringList* x_string_list_realloc(xStringList *lst, int capacity)
{
    xStringList *tmp = NULL;

    if( capacity <= lst->capacity )
        return lst;

    tmp = x_string_list_new( capacity );

    memcpy(tmp->list, lst->list, sizeof(lst->list[0])*lst->size);
    tmp->size = lst->size;

    lst->size = 0;

    x_string_list_free(lst);

    return tmp;
}

void x_string_list_add(xStringList** ptr, int count, ...)
{
    xStringList *lst = *ptr;
    xStringList *tmp = NULL;
    va_list par;
    int i;

    if( !lst )
    {
        assert("lst is a null pointer"==0);
        return;
    }

    if( lst->size == lst->capacity )
        lst = x_string_list_realloc(lst, 2*lst->capacity);

    lst->list[lst->size] = (StringList)malloc(sizeof(String)*(count+1));

    va_start(par, count);

    for(i = 0; i < count; i++)
        lst->list[lst->size][i] = strdup(va_arg(par, String)); 

    va_end(par);

    lst->list[lst->size][count]=NULL;

    ++lst->size;
    *ptr = lst;
}

void x_string_list_free(xStringList *lst)
{
    int i;
    int j;

    if( !lst )
    {
        assert("lst is a null pointer"==0);
        return;
    }

    for(i = 0; i < lst->size; i++) 
    {
        for(j = 0; lst->list[i][j]; ++j)
            free(lst->list[i][j]);

        free(lst->list[i]);
    } 

    free(lst);
}


main()
{
    xStringList *lst = x_string_list_new(1);

    x_string_list_add(&lst, 2, "1", "2");
    x_string_list_add(&lst, 2, "3", "4");

    x_string_list_free(lst);
}

Здравствуйте, Аноним, Вы писали:

А>Здравствуйте, Vamp, Вы писали:

V>>Ну что же...
V>>Даст ист рефакторинг!
V>>Я постарался максимально сохранить логику автора, только сделал программу чуть более читаемой и избавился от готу.

V>>

V>>#include <malloc.h>
V>>#include <string.h>
V>>#include <sys/varargs.h>

V>>typedef char* str;
V>>typedef str* strlst;


V>>typedef struct _xStringList {
V>>    strlst* list;
V>>    int size;

V>>    /*<private>*/
V>>    int al;
V>>} xStringList;

V>>void x_string_list_add(xStringList* lst, int count, ...);

V>>xStringList* x_string_list_new()
V>>{
V>>    xStringList *lst = (xStringList *)x_malloc(sizeof(xStringList)); //allocator's left as is
V>>    if (!lst)
V>>        return NULL;
    lst->>>list = (strlst*) x_malloc(sizeof(strlst) * 255);
V>>    if (!lst->list) {
V>>        free(lst);
V>>        return NULL;
V>>    }
    lst->>>al = 255;
    lst->>>size = 0;
V>>    return lst;
V>>}


V>>void x_string_list_add(xStringList* lst, int count, ...)
V>>{
V>>    strlst* tmp;
V>>    va_list par;
V>>    int i;
    
V>>    if (!lst) 
V>>    lst=x_string_list_new();
V>>    if (!lst)
V>>    return;
V>>    //logic left as is. Although you should mention, that such allocated list would be lost after
V>>    //function returns.        
    
V>>    if (lst->al <= lst->size) {
V>>    //again logic left as is. But we could consider using some form of x_realloc?
V>>    tmp = lst->list;
    lst->>>al*=2;
    lst->>>list = (strlst*)x_malloc(sizeof(strlst)*(lst->al));
V>>    if (!lst->list)
V>>        return;
V>>        for (i = 0; i < lst->size; ++i) 
           lst->>>list[i] = tmp[i];
V>>        x_free(tmp);
V>>    }
    
    lst->>>list[lst->size] = (strlst)x_malloc(sizeof(str) * (count+1));
V>>    va_start(par, count);
V>>    for (i = 0; i < count; i++)
         lst->>>list[lst->size][i] = x_strdup(va_arg(par, str)); 

    lst->>>list[lst->size][i]=NULL;
V>>    ++lst->size;
V>>    va_end(par);
V>>}

V>>void x_string_list_free(xStringList *lst)
V>>{
V>>    int i, j;
V>>    if (!lst)
V>>        return;
V>>    for (i = 0; i < lst->size; i++) {
V>>        for (j = 0; lst->list[i][j]; ++j)
V>>             x_free(lst->list[i][j]);
V>>         x_free(lst->list[i]);
V>>    } 
V>>  x_free(lst->list);
V>>  x_free(lst);
V>>}

V>>int main() {
V>>    xStringList* thelist=x_string_list_new();
V>>    x_string_list_add(thelist, 2, "one string", "another string");
V>>    x_string_list_free(thelist);
V>>    return 0;
V>>}
V>>

А>Класс! Именно так написал бы "Я" сегодняшний. Всё это дело было написано 7 лет назад, и только для фиксированного числа параметров. Сегодня понадобилось вновь, теперь для переменного.
Остановился на таком варианте:

/* x_string_list.h */
#ifndef __x_list_h_
#define __x_list_h_

#ifdef __cplusplus
extern "C" {
#endif

typedef struct _xStringList xStringList;

struct _xStringList
{    
    char ***list;

    int size;

    /*<private>*/
    int al;
};

xStringList *_x_string_list_new(void);

void x_string_list_add(xStringList *, int, ...);

xStringList *_x_string_list_free(xStringList *);

#define x_string_list_new() _x_string_list_new()
#define x_string_list_free(list) ((list) = _x_string_list_free((xStringList *)(list)))

#ifdef __cplusplus
}
#endif

#endif

/* x_string_list.c */
#include "x_lib.h"
#include <stdarg.h>

xStringList *_x_string_list_new(void)
{
    xStringList *lst = (xStringList *)x_malloc(sizeof(xStringList));
    lst->list = (char ***)x_malloc(sizeof(char **) * 255);
    lst->size = 0;
    lst->al = 255;
    return lst;
}
void x_string_list_add(xStringList *lst, int count, ...)
{
    va_list par;
    int i;
    if (!lst)    lst = x_string_list_new();
    if (lst->al <= lst->size)
    {
        char ***tmp;
        tmp = lst->list;
        lst->list = (char ***)x_malloc(sizeof(char **)*(lst->al*=2));
        for (i = 0; i < lst->size; i++) lst->list[i] = tmp[i];
        x_free(tmp);
    } 
    lst->list[lst->size] = (char **)x_malloc(sizeof(char *) * (count + 1));
    va_start(par, count);
    for (i = 0; i < count; i++)
    {
        lst->list[lst->size][i] = x_strdup(va_arg(par, char *));
    }
    lst->list[lst->size++][i] = NULL;
    va_end(par);
}

xStringList *_x_string_list_free(xStringList *lst)
{
    int i, j;
    if (lst != NULL)
    {
        for (i = 0; i < lst->size; i++)
        {
            for (j = 0; lst->list[i][j] != NULL; j++)
            {
                x_free(lst->list[i][j]);
            }    
            x_free(lst->list[i]);
        } 
        x_free(lst->list);
        x_free(lst);
    }
    return NULL;
}

	От:	Аноним
	Дата:	06.02.04 07:31
	Оценка:

	От:	Vamp
	Дата:	06.02.04 07:38
	Оценка:	+1

	От:	Аноним
	Дата:	06.02.04 07:42
	Оценка:

	От:	dik o-braz
	Дата:	06.02.04 08:14
	Оценка:

	От:	alexkro
	Дата:	06.02.04 08:26
	Оценка:

	От:	Анатолий Широков
	Дата:	06.02.04 10:01
	Оценка: