Czy w tej funkcji jest błąd?

0

Witajcie,

Debuguje program metodą jakiegoś jaskiniowca. (na Linux chyba nie ma dobrego debuggera, w Win to MSVC ma dobry). Czy w tej funkcji znajduje się błąd? Nie składniowy tylko działania. Kompiluje sie bez problemu tylko program rzuca mi błędami pamięci, teraz akurat takim *** Error in './bearchhost': double free or corruption (fasttop): 0x00000000009e75d0 *** Przerwane (zrzut pamięci) wcześniej był access violation, ale go znalazłem. Oto funkcja:

 void Node::addChild(Node* child)
{
	/* OBJASNIENIE, PROSZĘ PRZECZYTAĆ...
	 * header->itemChildrens = 0
	 * childrens = new Node[0];
	 * child to utworzona i uzupełniona wcześniej dynamicznie zaalokowana klasa. 
	 * Po wywolaniu tej funkcji to child jest juz usuwane
	 */

	Node* tempChildrens = new Node[header->itemChildrens + 1];
	std::memcpy(tempChildrens, childrens, header->itemChildrens*sizeof(Node));
	delete [] childrens;

	childrens = tempChildrens;
	childrens[header->itemChildrens] = *child;
	++header->itemChildrens;

	sort();
}

Oto klasa Node:

 class Node {
//private:
public:
	FileHeader* header;
	Node* childrens;
	Node* parent;
	static int compareNodes(Node* a, Node* b);
public:
	Node();
	~Node();

	void addChild(Node* child);
	...
	void sort();
	...
};

PS. Dodam może, że nadrzędną funkcją jest to:

Error Archive::mkDir(FilePath const& dirName)
{
	FileChildrenNum result = 0;
	Error err = actualNode->findChild(dirName, result);

	if(err == NOT_FOUND)
	{
		Node* tempNode = new Node();
		FileHeader* tempNodeHeader = 0;
		tempNode->getHeader() = tempNodeHeader = new FileHeader;

		std::memset(tempNodeHeader->itemName, 0, FileMaxName);
		std::strcpy(tempNodeHeader->itemName, dirName.c_str());
		tempNodeHeader->itemChildren = 0;
		tempNodeHeader->itemType = DIRECTORY;
		tempNodeHeader->dataFirst = 0;
		tempNodeHeader->dataLength = 0;

		actualNode->addChild(tempNode);
		delete tempNode;
	}
	else if(err == INVALID_PATH)
		return err;
	else if(err == IS_OK)
		return FILE_ALREADY_EXISTS;
	else
		return UNDEFINED_BEHAVIOR;

	return IS_OK;
}

Problem nie powstaje przy pierwszym wywołaniu mkDir tylko przy podwójnym wywołaniu tej funkcji (niezależnie od parametrów). Error w Linuxie to właśnie ten *** Error in './bearchhost': double free or corruption (fasttop): 0x00000000009e75d0 *** Przerwane (zrzut pamięci) wcześniej był access violation. Sprawdziłem wszystkie delete wywoływane przy funkcji i nie widze żadnych podwójnych.

2
  • Po wywolaniu tej funkcji to child jest juz usuwane

Jest jakiś powód takiego kopiowania obiektu i niszczenia oryginału?
Nie lepiej po prostu "zawłaszczyć" ten obiekt, czyli skopiować wskaźnik i nie niszczyć obiektu?

childrens

liczba mnoga od “child” to “children”. nie ma czegoś takiego jak “childrens”.

1

Troche wróżenie z fusów.
Masz napisane że gdzieś zwalniasz pamięć 2 razy.
Co do debuggerów to masz przecież na linucha np kolejno:

  • Nakładka na GDB w Code::Blocks
  • DDD, nieco stary ale da sie używać, też nakładka
  • No i samo GDB.
0

Moimi prehistorycznymi metodami doszedłem, że w funkcji mkDir w pierwszym wywołaniu delete tempNode; wywołuje się, a w drugim już nie i powoduje błąd double free. Tylko to są osobne zmienne lokalne i osobne miejsca w pamięci... Chyba, że optymalizacja kompilatora robi jakieś wtf.

PS. Te moje prehistoryczne metody to cout po ważnych momentach w kodzie i wyświetlanie jaki Error zwraca funkcja. Jak dojde do dokładnego miejsca w którym jest crash to między każdą linijką daje cout z informacją. Haha

2

przy błędach pamięciowych błąd klasy AV może Ci wyskoczyć w dowolnym miejscu.

Trochę lepsza wersja Twojego kodu (usuwa jeden potencjalny błąd):

 void Node::addChild(Node* child)
{
    size_t newSize = header->itemChildrens + 1;
    Node* tempChildrens = new Node[newSize];

    std::memcpy(tempChildrens, childrens, header->itemChildrens*sizeof(Node));

    std::swap(tempChildrens, childrens);
    delete [] tempChildrens;
 
    childrens[header->itemChildrens] = *child;
    header->itemChildrens = newSize;
 
    sort();
}

Edit: nie używaj strcpy, lepsze: strncpy.

0

Nadal jest ten błąd z usuwaniem tempNode w mkDir. Bardzo to dziwne. Przecież do Node zostaje włożona kopia tempNode, a wskaźnik nie jest zmieniany. Zastosowałem poprawkę @vpiotr, ale jednak błąd nie leży w dodawaniu nowego dziecka do Node. Zauważyłem, że w nowo stworzonym Node children jest NULL, a w destruktorze jest wywoływane delete [] children Już znalazłem winowajce pewnie. Zaraz sprawdzę.

PS. Błąd niby nadal jest.

0
bajos napisał(a):

PS. Błąd niby nadal jest.

Popraw jeszcze to:

tempNode->getHeader() = tempNodeHeader = new FileHeader;

(getHeader raczej nie jest do zmiany)

0

Funkcje w node tym który powoduje błąd wykonywane sa w tej kolejnosci. AddChildren jest wykonywane na nadrzędnym Node który z kolei takich błędów nie ma:

Node::Node()
{
	children = new Node[0];
	parent = 0;
	header = new FileHeader;
}

Node::~Node()
{
	/* Ta linijka ma blad */
	/* --->*/ delete [] children;
	/* Nie wiem jakim cudem to powoduje blad?
	 * Przecież nic nie jest usuwane/tworzone kilka razy jak sugeruje error */

	parent = 0;
	delete header;
}

Przecież to tak jak by napisać:

Node* children = new Node[0];
delete [] children;

Node* tempChildren = new Node[1];
delete [tempNode];
4

childrens[header->itemChildrens] = *child;
Nie masz operatora kopiowania, przez co wszystkie wskaźniki z klasy Node zostają przekopiowane, i jeśli w destruktorze masz delete[] wykonywane na nich, no to jak przekopiujesz tak jeden raz to 2 razy zwolnisz tą samą pamięć. A chyba nie muszę mówić, że zwalnianie tej samej pamięci w większych programach/grach jest koszmarem, bo może się zdarzyć tak, że ten sam obszar pamięci masz w dwóch wskaźnikach, na jednym zostanie wykonane delete[], na drugim zostanie wykonane później, ale w międzyczasie dokładnie ten sam obszar pamięci jest przekazywany innemu wskaźnikowi. I wtedy kompletnie "z d**y" nagle obszar, który dopiero co przydzieliliśmy jakiemuś wskaźnikowi zostaje magicznie zwolniony, i crashuje w losowych miejscach.

0

Wymyśliłem lepsze rozwiązanie. addChild() nie będzie przyjmowało parametru, a będzie zwracało wskaźnik na to dodane dziecko które będzie można ustawić według potrzeby. Może przeciążę operator= albo zrobię swap(). Przeczytałem na SO, że new[] allokuję sizeof(typ)*liczbaElementów+4 i przesuwa ten wskaźnik do przodu o sizeof(size_t). Ja będę musiał cofnąć ten wskaźnik na dzieci o cztery bajty i zrobić realloc() o rozmiarze sizeof(Node)*nowyRomiar+sizeof(size_t). Tylko wszystko ładnie, ale napisałem dla testu na http://ideone.com/DzAyGM , ale wynik zaalokowanej pamięci to 49. Nie powinno być sizef(int)*10+4 czyli 44? Jeszcze gryzie mnie czy w 64bitowym programie przed tym co zwraca new[] znajduje się liczba 4bajty czy 8bajtow? Tak to można by tylko alokować do 4GiB pamięci jednorazowo.

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