shithub: vcardfs

Download patch

ref: dc3098f8d9b9efa4fbcc88f11e9c758620e0397b
parent: 53f2de182c03080309f9661fd483877bcb8f21e8
author: sirjofri <sirjofri@sirjofri.de>
date: Tue Oct 22 06:20:45 EDT 2024

fixes memory leaks when deleting data

--- a/README.md
+++ b/README.md
@@ -46,10 +46,6 @@
 - import: update existing entries (how?)
 - files: output with newline at the end?
 
-Known bugs:
-
-- deleting files potentially leads to memory leaks, as the destroy function isn't called properly (see comment there)
-
 ### libvcard
 
 - see `libvcard/vcard.h`
--- a/vcardfs.c
+++ b/vcardfs.c
@@ -537,6 +537,7 @@
 		return dirf;
 	}
 	paramf->aux = emkvfile(Qparams, vf->card, nl, nil, vf->cardfile);
+	closefile(paramf);
 	
 	dataf = createfile(dirf, "data", user, 0666, nil);
 	if (!dataf) {
@@ -544,6 +545,8 @@
 		return dirf;
 	}
 	dataf->aux = emkvfile(Qdata, vf->card, nl, nil, vf->cardfile);
+	closefile(dataf);
+	closefile(dirf);
 	respond(r, nil);
 	return dirf;
 }
@@ -595,6 +598,9 @@
 		/* assume already responded */
 		return;
 	}
+	/* not sure if that's needed, but if I close the file, it seems
+	to clean up properly */
+	closefile(r->fid->file);
 	r->fid->file = nf;
 	r->ofcall.qid = nf->qid;
 	
@@ -661,15 +667,19 @@
 	File *file, *subfile;
 	Vparam *p;
 	
+	incref(f);
 	file = walkfile(f, "data");
 	if (file)
 		removefile(file);
+	incref(f);
 	file = walkfile(f, "group");
 	if (file)
 		removefile(file);
+	incref(f);
 	file = walkfile(f, "params");
 	if (file) {
 		for (p = l->params; p; p = p->next) {
+			incref(file);
 			subfile = walkfile(file, p->name);
 			if (subfile)
 				removefile(subfile);
@@ -687,8 +697,8 @@
 	for (l = c->content; l; l = l->next) {
 		if (!l->name)
 			continue;
-		file = walkfile(f, l->name);
 		incref(f);
+		file = walkfile(f, l->name);
 		if (file) {
 			rmclearlchildren(file, l);
 			removefile(file);
@@ -695,6 +705,7 @@
 		}
 	}
 	
+	incref(f);
 	file = walkfile(f, "export");
 	if (file)
 		removefile(file);
@@ -726,6 +737,7 @@
 	rmclearcchildren(f, c);
 	vcfreecards(c);
 	vf->card = nil;
+	/* on success, f will be removed automatically */
 	respond(r, nil);
 }
 
@@ -761,6 +773,7 @@
 	rmclearlchildren(f, l);
 	vcfreelines(l);
 	vf->line = nil;
+	/* on success, f will be removed automatically */
 	respond(r, nil);
 }
 
@@ -773,11 +786,7 @@
 	
 	if (l->params == p) {
 		l->params = p->next;
-		p->next = nil;
-		vcfreeparams(p);
-		f->param = nil;
-		respond(r, nil);
-		return;
+		goto Out;
 	}
 	
 	for (cp = l->params; cp; cp = cp->next)
@@ -789,9 +798,11 @@
 		return;
 	}
 	cp->next = p->next;
+Out:
 	p->next = nil;
 	vcfreeparams(p);
 	f->param = nil;
+	/* on success, f will be removed automatically */
 	respond(r, nil);
 }
 
@@ -904,7 +915,7 @@
 static void
 initcardfiles(Vcard *chain)
 {
-	File *fc, *fl, *f;
+	File *fc, *fl, *fp, *f;
 	Vcard *c;
 	Vline *l;
 	Vparam *p;
@@ -923,7 +934,8 @@
 		if (!fc)
 			sysfatal("%r");
 		vf = emkvfile(Qexport, c, nil, nil, cf);
-		createfile(fc, "export", user, 0444, vf);
+		f = createfile(fc, "export", user, 0444, vf);
+		closefile(f);
 		
 		for (l = c->content; l; l = l->next) {
 			vf = emkvfile(Qline, c, l, nil, cf);
@@ -936,23 +948,28 @@
 				s = nil;
 			}
 			vf = emkvfile(Qdata, c, l, nil, cf);
-			createfile(fl, "data", user, 0666, vf);
+			f = createfile(fl, "data", user, 0666, vf);
+			closefile(f);
 			if (l->group) {
 				vf = emkvfile(Qgroup, c, l, nil, cf);
-				createfile(fl, "group", user, 0666, vf);
+				f = createfile(fl, "group", user, 0666, vf);
+				closefile(f);
 			}
 			vf = emkvfile(Qparams, c, l, nil, cf);
-			f = createfile(fl, "params", user, DMDIR|0777, vf);
+			fp = createfile(fl, "params", user, DMDIR|0777, vf);
 			
 			for (p = l->params; p; p = p->next) {
 				vf = emkvfile(Qparamdata, c, l, p, cf);
-				createfile(f, p->name, user, 0666, vf);
+				f = createfile(fp, p->name, user, 0666, vf);
+				closefile(f);
 			}
+			closefile(fp);
+			closefile(fl);
 		}
+		closefile(fc);
 	}
 }
 
-/* TODO: for some reason, deleting leaf nodes like export doesn't call fdestroy */
 static void
 fdestroy(File *f)
 {
@@ -960,7 +977,9 @@
 	
 	vf = f->aux;
 	
-//	fprint(2, "destroy: %s\n", f->name);
+	/* File*s must be closefile'd properly after creation */
+	/* walkfile needs an incref, because it closefile's the dir */
+	//fprint(2, "destroy: %s\n", f->name);
 	
 	if (vf)
 		free(vf);
@@ -1010,7 +1029,7 @@
 		emkvfile(Qimport, nil, nil, nil, nil));
 	f = emkvfile(Qgexport, nil, nil, nil, nil);
 	f->cardfile = f;
-	createfile(fs.tree->root, "export", user, 0444, f);
+	closefile(createfile(fs.tree->root, "export", user, 0444, f));
 	initcardfiles(cards);
 	
 	postmountsrv(&fs, service, mtpt, MREPL);