Re: [Patch 3/4] chunkd: make error paths more regular

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/26/2009 01:34 AM, Pete Zaitcev wrote:
diff --git a/server/be-fs.c b/server/be-fs.c
index 89635f8..6551c83 100644
--- a/server/be-fs.c
+++ b/server/be-fs.c
@@ -75,7 +75,7 @@ int fs_open(void)
  	}

  	if (!tchdbsetmutex(hdb))
-		goto out_hdb;
+		goto out_mut;

  	omode = HDBOREADER | HDBONOLCK | HDBOWRITER | HDBOCREAT | HDBOTSYNC;
  	if (!tchdbopen(hdb, db_fn, omode)) {
@@ -86,24 +86,22 @@ int fs_open(void)

  	chunkd_srv.tbl_master = hdb;

-out:
  	free(db_fn);
-	return rc;
+	return 0;

+out_mut:
  out_hdb:
  	tchdbdel(hdb);
-	goto out;
+out:
+	free(db_fn);
+	return rc;
  }


This part is OK.


-void fs_free(void)
-{
-	if (chunkd_srv.tbl_master)
-		tchdbdel(chunkd_srv.tbl_master);
+	tchdbdel(chunkd_srv.tbl_master);
+	chunkd_srv.tbl_master = NULL;
  }

  bool fs_table_open(const char *user, const void *kbuf, size_t klen,
diff --git a/server/chunkd.h b/server/chunkd.h
index a18a6d4..d0b0ea6 100644
--- a/server/chunkd.h
+++ b/server/chunkd.h
@@ -220,7 +220,6 @@ extern struct hail_log cldu_hail_log;
  /* be-fs.c */
  extern int fs_open(void);
  extern void fs_close(void);
-extern void fs_free(void);
  extern struct backend_obj *fs_obj_new(uint32_t table_id, const void *kbuf, size_t klen,
  				      enum chunk_errcode *err_code);
  extern struct backend_obj *fs_obj_open(uint32_t table_id, const char *user,
diff --git a/server/server.c b/server/server.c
index 381772b..1b48905 100644
--- a/server/server.c
+++ b/server/server.c
@@ -1664,13 +1664,13 @@ int main (int argc, char *argv[])

  	if (fs_open()) {
  		rc = 1;
-		goto err_out_session;
+		goto err_out_fs;
  	}

  	if (cld_begin(chunkd_srv.ourhost, chunkd_srv.cell, chunkd_srv.nid,
  		&chunkd_srv.loc, NULL)) {
  		rc = 1;
-		goto err_out_fs;
+		goto err_out_cld;
  	}

  	/* set up server networking */
@@ -1686,19 +1686,17 @@ int main (int argc, char *argv[])

  	applog(LOG_INFO, "shutting down");

+	/* net_close(); */
  err_out_listen:
  	cld_end();
-err_out_fs:
+err_out_cld:
  	fs_close();
+err_out_fs:
+	g_hash_table_destroy(chunkd_srv.fd_info);
  err_out_session:
-	/* net_close(); */
  	unlink(chunkd_srv.pid_file);
  	close(chunkd_srv.pid_fd);
  err_out:
-	if (strict_free) {
-		fs_free();
-		g_hash_table_destroy(chunkd_srv.fd_info);
-	}


I think it was misunderstood what is going on here. Read first what strict_free does, the docs are in "chunkd --help" output.

        { "strict-free", 1001, NULL, 0,
          "For memory-checker runs.  When shutting down server, free local "
          "heap, rather than simply exit(2)ing and letting OS clean up." },

It can be significantly faster to exit() rather than needlessly walk data structures and free them, when the OS can do the same thing, only faster and with more efficiency.

Therefore the end code in main() handles both normal shutdown, after possibly considerable runtime has elapsed, and error shutdown. Toss into the mix memory-checker code, and the result can be quite confusing, I admit.

The only time we need to free heap objects is the rare, non-default case of memory checker (valgrind, here) runs.

	Jeff




--
To unsubscribe from this list: send the line "unsubscribe hail-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Fedora Clound]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux