C++

Forum

 
Memory leak - Can you find it?

roger911 (127)
Hey guys, I'm working on building a map maker for learning purposes, and decided to start building the main menu in SDL. I have a button called "new" that pops up a new window which is intended to get parameters of a new map, but this window has a memory leak issue and I can't seem to find it.

The memory leak happens when closing the "new window" and opening it again. It seems as though the destroy function I build isn't destroying everything I created, but I don't see what I'm missing.

List of associated functions:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
 
void Button_New()
{
	if(init_defineNew() != true)
	{
		cout << "Failed to initialize assets for NEW" << endl;
	}
	else
	{
		if(load_NWin() == false)
		{
			cout << "Failed to load NWin assets"<<endl;
		}
		NWin_active = true;
	}
}

bool init_defineNew()
{
	bool gflag = true;
	NWin = SDL_CreateWindow("Sidewin", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, NWx, NWy, SDL_WINDOW_SHOWN);	
	if(NWin == NULL)
	{
		cout << "Failed to create new window" << endl;
		gflag = false;
	}
	else
	{
		NWinren = SDL_CreateRenderer(NWin, -1, SDL_RENDERER_ACCELERATED);	
	}
        return gflag;
}

bool load_NWin()
{
	rtemp.x = 0;
	rtemp.y = 0;
	rtemp.w = NWx;
	rtemp.h = NWy;
	NWintex = Make_Texture_for_Rect(&rtemp, 0x7D, 0x7D, 0x7D, NWinren);
	//Assigning rect data
	fillrect();
	bool gflag = true;
	NWfTitle = lFontToTexture("Enter map parameters", NWinren);
	MapName = lFontToTexture("Name", NWinren);
	MapHeight = lFontToTexture("Height", NWinren);
	MapWidth = lFontToTexture("Width", NWinren);
	return gflag;
}

void fillrect()
{
	if(if_init == false)
	{
		f1.x = 150;
		f1.y = 50;
		f1.h = 26;
		f1.w = 100;
		f2.x = 150;
		f2.y = 100;
		f2.h = 26;
		f2.w = 100;
		f3.x = 150;
		f3.y = 150;
		f3.h = 26;
		f3.w = 100;
		t1.x = 0;
		t1.y = 50;
		t1.h = 26;
		t1.w = 100;
		t2.x = 0;
		t2.y = 100;
		t2.h = 26;
		t2.w = 100;
		t3.x = 0;
		t3.y = 150;
		t3.h = 26;
		t3.w = 100;
		NMWfT.x = 0;
		NMWfT.y = 0;
		NMWfT.h = 26;
		NMWfT.w = 100;
		iMapName = Make_Texture_for_Rect(&f1, 0xC3, 0xC3, 0xC3, NWinren);
		iMapHeight = Make_Texture_for_Rect(&f2, 0xC3, 0xC3, 0xC3, NWinren);
		iMapWidth = Make_Texture_for_Rect(&f3, 0xC3, 0xC3, 0xC3, NWinren);
		if_init = true;
	}
	if(if_init == true && has_clicked == true)
	{
		if(sinp.active_array == 0)
		{
			SDL_DestroyTexture(iMapName);
			SDL_DestroyTexture(iMapWidth);
			SDL_DestroyTexture(iMapHeight);
			iMapName = Make_Texture_for_Rect(&f1, 0x9B, 0x9B, 0x9B, NWinren);
			iMapHeight = Make_Texture_for_Rect(&f2, 0xC3, 0xC3, 0xC3, NWinren);
			iMapWidth = Make_Texture_for_Rect(&f3, 0xC3, 0xC3, 0xC3, NWinren);
			has_clicked = false;
		}
		if(sinp.active_array == 1)
		{
			SDL_DestroyTexture(iMapName);
			SDL_DestroyTexture(iMapWidth);
			SDL_DestroyTexture(iMapHeight);
			iMapName = Make_Texture_for_Rect(&f1, 0xC3, 0xC3, 0xC3, NWinren);
			iMapHeight = Make_Texture_for_Rect(&f2, 0x9B, 0x9B, 0x9B, NWinren);
			iMapWidth = Make_Texture_for_Rect(&f3, 0xC3, 0xC3, 0xC3, NWinren);
			has_clicked = false;
		}
		if(sinp.active_array == 2)
		{
			SDL_DestroyTexture(iMapName);
			SDL_DestroyTexture(iMapWidth);
			SDL_DestroyTexture(iMapHeight);
			iMapName = Make_Texture_for_Rect(&f1, 0xC3, 0xC3, 0xC3, NWinren);
			iMapHeight = Make_Texture_for_Rect(&f2, 0xC3, 0xC3, 0xC3, NWinren);
			iMapWidth = Make_Texture_for_Rect(&f3, 0x9B, 0x9B, 0x9B, NWinren);
			has_clicked = false;
		}
	}
}

SDL_Texture* loadTexture(string path, SDL_Renderer *target_render)
{
	SDL_Texture* newtex = NULL;
	img = SDL_LoadBMP(path.c_str());
	if(img == NULL)
	{
		cout << "Could not load img" << endl;
	}
	else
	{
		newtex = SDL_CreateTextureFromSurface(target_render, img);
		if(newtex == NULL)
		{
			cout << "Failed to load img to a texture" << endl;
		}
		else
		{
			SDL_FreeSurface(img);
		}
		return newtex;
	}
}
SDL_Texture* lFontToTexture(const char *text, SDL_Renderer *target_render)
{
	SDL_Texture* newtex = NULL;
	img = TTF_RenderText_Solid(ubuntu24, text, fClr); //Applying the font to a surface;
	if(img == NULL)
	{
		cout << "Could not load img" << endl;
	}
	else
	{
		newtex = SDL_CreateTextureFromSurface(target_render, img);
		if(newtex == NULL)
		{
			cout << "Failed to load img to a texture" << endl;
		}
		else
		{
			SDL_FreeSurface(img);
                        return newtex;
		}
	}
}

SDL_Texture* Make_Texture_for_Rect(SDL_Rect *rect, Uint32 Rcolor, Uint32 Gcolor, Uint32 Bcolor, SDL_Renderer *target_render)
{
	SDL_Surface *temp = NULL;
	temp = SDL_LoadBMP("bax2.bmp");
	SDL_Texture *newtex;
	SDL_FillRect(temp, NULL, SDL_MapRGB(temp->format, Rcolor, Gcolor, Bcolor));
	newtex = SDL_CreateTextureFromSurface(target_render, temp);
	SDL_FreeSurface(temp);
	return newtex;
}

void render_nscreen()
{
	SDL_RenderClear(NWinren);
	SDL_RenderCopy(NWinren, NWintex, NULL, NULL);
	SDL_RenderCopy(NWinren, iMapName, NULL, &f1);
	SDL_RenderCopy(NWinren, iMapHeight, NULL, &f2);
	SDL_RenderCopy(NWinren, iMapWidth, NULL, &f3);
	SDL_RenderCopy(NWinren, NWfTitle, NULL, &NMWfT);
	SDL_RenderCopy(NWinren, MapName, NULL, &t1);
	SDL_RenderCopy(NWinren, MapHeight, NULL, &t2);
	SDL_RenderCopy(NWinren, MapWidth, NULL, &t3);
	SDL_RenderPresent(NWinren);
}

void destroy_win(SDL_Window *wparam)
{
	if(wparam == NWin)
	{
		if_init = false;
		NWin_active = false;
		SDL_DestroyTexture(NWintex);
		SDL_DestroyTexture(NWfTitle);
		SDL_DestroyTexture(MapName);
		SDL_DestroyTexture(MapHeight);
		SDL_DestroyTexture(MapWidth);
		SDL_DestroyTexture(iMapName);
		SDL_DestroyTexture(iMapWidth);
		SDL_DestroyTexture(iMapHeight);
		SDL_DestroyRenderer(NWinren);
		SDL_DestroyWindow(NWin);
	}
}


I know it's a pain looking through 200 lines of random functions, but if someone could help me find the cause of the leak that would be super awesome.

Edit : Thinking about it feels like I'm playing a game of find waldo
Last edited on
cire (8284)
What makes you think you have a leak?

What purpose does the local-to-Button_New variable gflag serve?

In init_defineNew why don't you return a value as promised by the function prototype?

In lFontToTexture why don't you return a value as promised by the function prototype?

In loadTexture why don't you return a value as promised by the function prototype?

Why are you repeatedly loading the same texture over and over with Make_Texture_for_Rect?

In loadTexture and lFontToTexture you use a global variable img which can fail to be deallocated in those functions and will then be overwritten by the next call to either of those functions.

What purpose does the local-to-lFontToTexture variable gflag serve?

What purpose does the local-to-loadTexture variable gflag serve?
Last edited on
roger911 (127)
I know i have a leak, because I was monitering the resources the program took while running it. It adds about 3 megs of ram used each time i close and re-open the window (or use the Button_New function.

the gflags are bool variables that are returned in initializing functions. I put them in to pretty much every function by habit, and I haven't gone through the code to clean it up.

In all the functions which you're asking why they don't return values, they do, but some - only if there were no errors.

Their individual textures because they have different offsets, and depending on certain inputs, they individually change their characteristics. I don't see any other way of changing the color of say texture a which gets loaded at rect a, while keeping texture b and c the same color.

For loadTexture, and lFontToTexture, you suggest deallocating it regardless of success or fail of the function? I would think that if the function didn't complete properly in the 1st place that overwriting it in the next call wouldn't do any extra harm. However, it's good practice so I'll take your advice.

I'll clean try to clean up the code though, it is pretty messy.
Last edited on
cire (8284)
I know i have a leak, because I was monitering the resources the program took while running it. It adds about 3 megs of ram used each time i close and re-open the window (or use the Button_New function.

That doesn't necessarily mean you have a leak. And if you do, it may not be in this code.


In all the functions which you're asking why they don't return values, they do, but some - only if there were no errors.

In all the functions I asked why they don't return values, there are situations where they do not. If you have a function which promises to return a value and then does not, undefined behavior happens. You shouldn't care for undefined behavior in your programs.
Last edited on
roger911 (127)
Well, additional memory is being allocated continuously each time the Button_New function is run, and when the window closes by using the destroy_win function, it does not free up as much memory as added by the Button_New Function. I don't know how else to describe it other than a memory leak.

Interesing, I guess I picked up some bad habits. Thanks for pointing it out.
Porkhead (1)
This is a bug in SDL2.
closed account (o1vk4iN6)
Check the date when this was posted, and no it isn't there's a lot of places his code can go wrong and cause memory leaks.
Topic archived. No new replies allowed.