Naruszenie pamięci - struktura kontenera z danymi void*

0

Witam,
Mam problem z moim programem który w jednej strukturze MY_ARRAY przechowuje dane typu void*. Podczas wywołania programu podczas wyprowadzania danych to konsoli dostaję naruszenie dostępu pamięci w ostatniego elementu który chcę wydrukować i wyskakuje mi to powiadomienie przy funkcji printPoint. Nie wiem co tu może być nie tak z góry dzięki za pomoc.
data.h

typedef struct MY_POINT {
	double x, y;
	char* name;
}Point;

Point* initPoint(double a, double b, char* pointName);
void printPoint(void* ptr);
void freePoint(void* ptr);

data.cpp

#include "my_data.h"
#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#pragma warning (disable : 4996)

Point* initPoint(double a, double b, char* pointName){
	int len = strlen(pointName) + 1;

	Point* ptr = (Point*)malloc(sizeof(Point));
	if (!ptr)
		return NULL;

	ptr->name = (char*)malloc(len* sizeof(char));
	if (!ptr->name) {
		printf("Allocation error for name\n");
		return NULL;
	}
	memset(ptr->name, 0, len * sizeof(char));
	strcpy(ptr->name, pointName);
	//ptr->name[len] = '\n';
	ptr->x = a;
	ptr->y = b;

	return ptr;
}

void printPoint(void* ptr) {
	Point* tmp = (Point*)ptr;
	printf("Title: %s\nx= %lf, y=%lf\n", tmp->name, tmp->x, tmp->y);
}

void freePoint(void* ptr){
	Point* tmp = (Point*)ptr;
	if (tmp) {
		if (tmp->name)
			free(tmp->name);
		tmp->name = NULL;
		free(tmp);
	}
	tmp = NULL;
}


tab.h

#pragma once

typedef void (*PtrPrint)(void* data);
typedef void (*FreePtr)(void* ptr);

typedef struct MY_TAB {
	void* data;
	static size_t availablePosition;
}Array;

Array* initContainer(int dim, FreePtr fptr, PtrPrint pptr);
int addToContainer(Array* ptr, void* data);
void printContainer(Array* ptr);
void freeContainer(Array* ptr);

tab.cpp

#include "my_tab.h"
#include <stdlib.h>
#include <string.h>
#include <stdio.h>

size_t Array::availablePosition = 0;
FreePtr freeFunc;
PtrPrint printFunc;

Array* initContainer(int dim, FreePtr fptr, PtrPrint pptr){
	Array* newptr = (Array*)malloc(dim * sizeof(Array));
	if (!newptr) {
		printf("Container allocation error\n");
		return NULL;
	}
	memset((void*)newptr, 0, dim * sizeof(Array));
	freeFunc = fptr;
	printFunc = pptr;
	Array::availablePosition = 0;
	return newptr;
}

int addToContainer(Array* ptr, void* pdata){
	size_t len = _msize(ptr) / sizeof(Array);
	//printf("Len from add to container: %d\n", len);
	//printf("Available position from add to container: %zu\n", Array::availablePosition);
	if (Array::availablePosition >= len) {
		ptr = (Array*)realloc(ptr, len * 2 * sizeof(Array));
		if (!ptr)
			return 0;
	}
	ptr[Array::availablePosition].data = pdata;
	++Array::availablePosition;
	return 1;
}

void printContainer(Array* ptr){
	size_t i;
	//printf("Array len= %zu\n", Array::availablePosition);
	for (i = 0; i <= Array::availablePosition; i++)
		printFunc(ptr[i].data);
}

void freeContainer(Array* ptr){
	if (ptr) {
		for (size_t i = 0; i < Array::availablePosition; i++)
			freeFunc(ptr[i].data);
		Array::availablePosition = 0;
		free(ptr);
	}
	ptr = NULL;

}

main.cpp

#include <stdio.h>
#include "my_data.h"
#include "my_tab.h"


int main(void) {
	Point* t = initPoint(2.34, 4.67, (char*)"Asdasd");
	Point* t2 = initPoint(6.98, 123.45, (char*)"Bsada");
	Point* t3 = initPoint(34.98, 13.45, (char*)"Cdaas");

	Array* tab = initContainer(2, freePoint, printPoint);

	addToContainer(tab, t);
	addToContainer(tab, t2);
	addToContainer(tab, t3);
	printContainer(tab);
	freeContainer(tab);
	//printPoint(t);
	//printPoint(t2);

	return 0;
}
2

Tak na moje oko, addToContainer powinno przyjmować Array** ptr, ponieważ obecnie wskaźnik przed i po wywołaniu addToContainer pozostaje (z zewnątrz) taki sam, choć wewnątrz go realokujesz.

No i powinieneś (przynajmniej jeszcze) pozbyć się size_t Array::availablePosition = 0; na rzecz ptr->availablePosition oraz wewnątrz Array trzymać FreePtr + PrintPtr; w innym wypadku utworzenie drugiej kolekcji o innych funkcjach zepsuje pierwszą kolekcję.

1

Coś w ten deseń (zwyczajowo nie piszę w C, więc kod może się nie kompilować od strzała :-P):

typedef struct MY_TAB {
	void* data;
    size_t len;
	size_t capacity;
} Array;

int addToContainer(Array** ptr, void* pdata){
    if (*ptr->len >= *ptr->capacity) {
        *ptr->capacity *= 2;
        *ptr = /* realloc */;
    }

	*ptr.data[*ptr->len] = pdata;
    *ptr.len += 1;

	return 1;
}
1

Widzę w tym kodzie ogromnie dużo rozpaczy.

  • Memsetowanie po malloc, zamiast użyć calloc
  • rzutowanie tylko w konsekwencji swoich błędnych wyborów
  • (char*)"Asdasd" nagroda Nobla (albo Darwina)

Nawet JESLI dopadniesz jakieś wystąpienie błędu, wiele niewykrytych się nadal czai, przy takim chaotycznym kodowaniu

1
fatal(int bad,const char *message)  { if(bad) exit(printf("%s\n",message)); }
char *strduplicate(const char *str)
{
  int len=strlen(str)+1;
  char *ret=(char*)malloc(len); // char zawsze ma rozmiar 1,
  fatal(!ret,"Allocation error for c-string");
  memcpy(ret,str,len); // nie trzeba robić memset + strcpy
}

...
	ptr->name=strduplicate(pointName); 
    ptr->x = a; // przy takim nazewnictwie przy każdym wywołaniu initPoint będziesz
	ptr->y = b; // musiał znaleźć tą initPoint i sprawdzić czy `a` to `x` czy `y`
    // jak sobie pościelisz ...
0
_13th_Dragon napisał(a):
...
	ptr->name=strduplicate(pointName); 
    ptr->x = a; // przy takim nazewnictwie przy każdym wywołaniu initPoint będziesz
	ptr->y = b; // musiał znaleźć tą initPoint i sprawdzić czy `a` to `x` czy `y`
    // jak sobie pościelisz ...

Co masz na myśli przy ptr->x = a itd.? W jaki inny sposób mogę to w takim razie zrealizować.

0
Point* initPoint(double x, double y, char* pointName) // tu w podano że parametry są x,y i każde współczesne IDE poda to w podpowiedzi
{
		...
	return ptr;
}
0

@_13th_Dragon:
Czy masz może jakiejś sugestie do tego żeby ten program działał, muszę mieć to działające na jutro i już nie wiem w co łapy włożyć.

1

Sugestie są proste rozbij to na mniejsze funkcje sensownie nazwane z sensownie nazwanymi parametrami a sam zobaczysz problem.

1

nie mam niestety czasu na analizę ale fragment wrzuciłem w clang-tidy i cppcheck masz przykład np.
w initPoint
Cppcheck: (error) memleak: Memory leak: ptr konkretnie

    if (!ptr->name) {
        printf("Allocation error for name\n");
        return NULL;
    }

co ci polecam zrobić:

  1. Poprawić error/warningi kompilatora
  2. analiza statyczna np. cppcheck i inne
  3. Analiza debuggerem i innymi narzędziami jak aplikacja hula jak działa coś typu valgrinda, gdb itd.
0

Komp mi się zawiesił i skasowało mi cały post.napisałem tyle rzeczy ok. Super szybko.

Jak już poprawisz to co Ci napisał patryk to...

do double %f nie %lf ( lf to long double )
Nie używaj _msize na przyszłosć.
Sprawdź co _msize zwraca bo jak wywali Ci zero to masz katastrofe ( realloc biorąc nawet NULL i zero zwróci Ci jakiś adres.)
decyduj się czy C czy C++
Nie sprawdzasz nawet czy funkcja wywaliłą Ci błąd.
if( fun(x,y) == - 1 ) fail( "shit" );
Nie ma tego.
if( ! ptr ) return 0; Chyba raczej -1 jak ptr = NULL prawda ?
Zakumpluj się z assert.
Mozesz sobie powstawaić puts("doszło do tego punktu" ); To ułatwia. Choć ktoś pewnie powie, że to dość prymitywny sposób ;)

Ta globalna trzymająca funkcję drukującą która jest inicjalizowana, to jest naprawdę, grube.

1
barti_25 napisał(a):

tab.cpp

void printContainer(Array* ptr){
	size_t i;
	//printf("Array len= %zu\n", Array::availablePosition);
	for (i = 0; i <= Array::availablePosition; i++)
		printFunc(ptr[i].data);
}

void freeContainer(Array* ptr){
	if (ptr) {
		for (size_t i = 0; i < Array::availablePosition; i++)
			freeFunc(ptr[i].data);
		Array::availablePosition = 0;
		free(ptr);
	}
	ptr = NULL;

}

Masz pros­ty błąd w li­nii 4: indeks ava­ila­b­le­Po­si­tion nie is­tnieje, a po­ró­w­nujesz dla ró­w­no­ś­ci. Zo­bacz po­niżej w fun­kcji fre­e­Con­ta­i­ner.
Poza tym pisz w C lub C++.
_msize jest nieprzenośne, u mnie jest malloc_usable_size.

1 użytkowników online, w tym zalogowanych: 0, gości: 1