config: use std::string in md_config_t

Use std::string to represent md_config_t strings. This makes memory management a lot easier and should fix some leaks. "No value" is now represented by an empty string, whereas before some places were using empty strings and some were using NULL. config.cc: Fix a minor decode bug. In pid_file.cc, copy the pid_file using snprintf, since strncpy does not always NULL-terminate. Signed-off-by: Colin McCabe <[email protected]>
dano · Mar 30, 2011 · 9cde1f4 · 9cde1f4
1 parent 3c94cc2
commit 9cde1f4
Show file tree

Hide file tree

Showing 27 changed files with 125 additions and 178 deletions.
diff --git a/src/auth/KeyRing.cc b/src/auth/KeyRing.cc
@@ -160,13 +160,13 @@ void KeyRing::decode(bufferlist::iterator& bl) {
   }
 }
 
-int KeyRing::load(const char *filename)
+int KeyRing::load(const std::string &filename)
 {
-  if (!filename)
+  if (filename.empty())
     return -EINVAL;
 
   bufferlist bl;
-  int ret = bl.read_file(filename, true);
+  int ret = bl.read_file(filename.c_str(), true);
   if (ret < 0) {
     derr << "error reading file: " << filename << dendl;
     return ret;

diff --git a/src/auth/KeyRing.h b/src/auth/KeyRing.h
@@ -28,7 +28,7 @@ class KeyRing {
 public:
   map<EntityName, EntityAuth>& get_keys() { return keys; }  // yuck
 
-  int load(const char *filename);
+  int load(const std::string &filename);
   void print(ostream& out);
 
   // accessors

diff --git a/src/cfuse.cc b/src/cfuse.cc
@@ -72,7 +72,7 @@ int main(int argc, const char **argv, const char *envp[]) {
   vec_to_argv(nargs, argc, argv);
 
   // FUSE will chdir("/"); be ready.
-  g_conf.chdir = strdup("/");
+  g_conf.chdir = "/";
 
   // check for 32-bit arch
   if (sizeof(long) == 4) {
@@ -125,7 +125,7 @@ int main(int argc, const char **argv, const char *envp[]) {
 
     // start up fuse
     // use my argc, argv (make sure you pass a mount point!)
-    int r = client->mount(g_conf.client_mountpoint);
+    int r = client->mount(g_conf.client_mountpoint.c_str());
     if (r < 0) {
       cerr << "cfuse[" << getpid() << "]: ceph mount failed with " << strerror(-r) << std::endl;
       goto out_shutdown;

diff --git a/src/client/Client.cc b/src/client/Client.cc
@@ -65,7 +65,7 @@ using namespace std;
 #undef dout_prefix
 #define dout_prefix *_dout << "client" << whoami << " "
 
-#define  tout       if (g_conf.client_trace) traceout
+#define  tout       if (!g_conf.client_trace.empty()) traceout
 
 
 // static logger
@@ -2945,7 +2945,7 @@ void Client::handle_cap_grant(Inode *in, int mds, InodeCap *cap, MClientCaps *m)
 // -------------------
 // MOUNT
 
-int Client::mount(const char *mount_root)
+int Client::mount(const std::string &mount_root)
 {
   Mutex::Locker lock(client_lock);
 
@@ -2978,7 +2978,8 @@ int Client::mount(const char *mount_root)
   //  fuse assumes it's always there.
   MetaRequest *req = new MetaRequest(CEPH_MDS_OP_GETATTR);
   filepath fp(CEPH_INO_ROOT);
-  if (mount_root) fp = filepath(mount_root);
+  if (!mount_root.empty())
+    fp = filepath(mount_root.c_str());
   req->set_filepath(fp);
   req->head.args.getattr.mask = CEPH_STAT_CAP_INODE_ALL;
   int res = make_request(req, -1, -1);
@@ -2990,8 +2991,8 @@ int Client::mount(const char *mount_root)
   _ll_get(root);
 
   // trace?
-  if (g_conf.client_trace) {
-    traceout.open(g_conf.client_trace);
+  if (!g_conf.client_trace.empty()) {
+    traceout.open(g_conf.client_trace.c_str());
     if (traceout.is_open()) {
       dout(1) << "opened trace file '" << g_conf.client_trace << "'" << dendl;
     } else {
@@ -3105,7 +3106,7 @@ int Client::unmount()
   }
 
   // stop tracing
-  if (g_conf.client_trace) {
+  if (!g_conf.client_trace.empty()) {
     dout(1) << "closing trace file '" << g_conf.client_trace << "'" << dendl;
     traceout.close();
   }

diff --git a/src/client/Client.h b/src/client/Client.h
@@ -1222,7 +1222,7 @@ class Client : public Dispatcher {
 		    Dentry **pdn, bool expect_null=false);
 
 public:
-  int mount(const char *mount_root = NULL);
+  int mount(const std::string &mount_root);
   int unmount();
 
   // these shoud (more or less) mirror the actual system calls.

diff --git a/src/client/SyntheticClient.cc b/src/client/SyntheticClient.cc
@@ -310,7 +310,7 @@ int SyntheticClient::run()
   dout(15) << "initing" << dendl;
   client->init();
   dout(15) << "mounting" << dendl;
-  int err = client->mount();
+  int err = client->mount("");
   if (err < 0) {
     char buf[80];
     dout(0) << "failed to mount: " << strerror_r(-err, buf, sizeof(buf)) << dendl;

diff --git a/src/cmon.cc b/src/cmon.cc
@@ -73,28 +73,28 @@ int main(int argc, const char **argv)
       usage();
   }
 
-  if (!g_conf.mon_data) {
+  if (g_conf.mon_data.empty()) {
     cerr << "must specify '--mon-data=foo' data path" << std::endl;
     usage();
   }
 
   // -- mkfs --
   if (mkfs) {
-    if (!g_conf.monmap || !osdmapfn)
+    if (g_conf.monmap.empty() || !osdmapfn)
       usage();
 
     // make sure it doesn't already exist
         /*
     struct stat st;
-    if (::lstat(g_conf.mon_data, &st) == 0) {
+    if (::lstat(g_conf.mon_data.c_str(), &st) == 0) {
       cerr << "monfs dir " << g_conf.mon_data << " already exists; remove it first" << std::endl;
       usage();
     }
 	*/
 
     // load monmap
     bufferlist monmapbl, osdmapbl;
-    int err = monmapbl.read_file(g_conf.monmap);
+    int err = monmapbl.read_file(g_conf.monmap.c_str());
     if (err < 0)
       exit(1);
     MonMap monmap;

diff --git a/src/common/ClassHandler.cc b/src/common/ClassHandler.cc
@@ -26,7 +26,8 @@ int ClassHandler::_load_class(ClassData &cls)
   cls_deps_t *(*cls_deps)();
 
   char fname[80];
-  snprintf(fname, sizeof(fname), "%s/class-XXXXXX", g_conf.osd_class_tmp);
+  snprintf(fname, sizeof(fname), "%s/class-XXXXXX",
+	   g_conf.osd_class_tmp.c_str());
 
   int fd = mkstemp(fname);
   if (fd < 0) {

diff --git a/src/common/DoutStreambuf.cc b/src/common/DoutStreambuf.cc
@@ -50,24 +50,6 @@ static bool fd_is_open(int fd)
   return (res == 0);
 }
 
-static bool empty(const char *str)
-{
-  if (!str)
-    return true;
-  if (!str[0])
-    return true;
-  return false;
-}
-
-static string cpp_str(const char *str)
-{
-  if (!str)
-    return "(NULL)";
-  if (str[0] == '\0')
-    return "(empty)";
-  return str;
-}
-
 static std::string normalize_relative(const char *from)
 {
   if (from[0] == '/')
@@ -279,7 +261,7 @@ void DoutStreambuf<charT, traits>::read_global_config()
     }
   }
 
-  if ((!empty(g_conf.log_file)) || (!empty(g_conf.log_dir))) {
+  if ((!g_conf.log_file.empty()) || (!g_conf.log_dir.empty())) {
     if (_read_ofile_config() == 0) {
       flags |= DOUTSB_FLAG_OFILE;
     }
@@ -378,8 +360,8 @@ std::string DoutStreambuf<charT, traits>::config_to_str() const
   ostringstream oss;
   oss << "g_conf.log_to_stderr = " << g_conf.log_to_stderr << "\n";
   oss << "g_conf.log_to_syslog = " << g_conf.log_to_syslog << "\n";
-  oss << "g_conf.log_file = '" << cpp_str(g_conf.log_file) << "'\n";
-  oss << "g_conf.log_dir = '" << cpp_str(g_conf.log_dir) << "'\n";
+  oss << "g_conf.log_file = '" << g_conf.log_file << "'\n";
+  oss << "g_conf.log_dir = '" << g_conf.log_dir << "'\n";
   oss << "g_conf.g_conf.log_per_instance = '"
       << g_conf.log_per_instance << "'\n";
   oss << "flags = 0x" << std::hex << flags << std::dec << "\n";
@@ -443,15 +425,15 @@ std::string DoutStreambuf<charT, traits>::_calculate_opath() const
   // should hold the dout_lock here
 
   // If g_conf.log_file was specified, that takes the highest priority
-  if (!empty(g_conf.log_file)) {
-    return normalize_relative(g_conf.log_file);
+  if (!g_conf.log_file.empty()) {
+    return normalize_relative(g_conf.log_file.c_str());
   }
 
   string log_dir;
-  if (empty(g_conf.log_dir))
+  if (g_conf.log_dir.empty())
     log_dir = normalize_relative(".");
   else
-    log_dir = normalize_relative(g_conf.log_dir);
+    log_dir = normalize_relative(g_conf.log_dir.c_str());
 
   if (g_conf.log_per_instance) {
     char hostname[255];
@@ -478,8 +460,8 @@ std::string DoutStreambuf<charT, traits>::_calculate_opath() const
 template <typename charT, typename traits>
 std::string DoutStreambuf<charT, traits>::_get_symlink_dir() const
 {
-  if (!empty(g_conf.log_sym_dir))
-    return normalize_relative(g_conf.log_sym_dir);
+  if (!g_conf.log_sym_dir.empty())
+    return normalize_relative(g_conf.log_sym_dir.c_str());
   else
     return get_dirname(opath);
 }
@@ -499,7 +481,7 @@ int DoutStreambuf<charT, traits>::_read_ofile_config()
     return 1;
   }
 
-  if (empty(g_conf.log_file) && g_conf.log_per_instance) {
+  if (g_conf.log_file.empty() && g_conf.log_per_instance) {
     // Calculate instance symlink path (isym_path)
     ostringstream iss;
     std::string symlink_dir(_get_symlink_dir());

diff --git a/src/common/ProfLogger.cc b/src/common/ProfLogger.cc
@@ -194,7 +194,8 @@ void ProfLogger::_open_log()
   struct stat st;
 
   filename = "";
-  if (g_conf.chdir && g_conf.chdir[0] && g_conf.profiling_logger_dir[0] != '/') {
+  if ((!g_conf.chdir.empty()) &&
+      (g_conf.profiling_logger_dir.substr(0,1) != "/")) {
     char cwd[PATH_MAX];
     char *c = getcwd(cwd, sizeof(cwd));
     assert(c);
@@ -209,7 +210,7 @@ void ProfLogger::_open_log()
     ::mkdir(filename.c_str(), 0750);
 
   filename += "/";
-  if (g_conf.profiling_logger_subdir) {
+  if (!g_conf.profiling_logger_subdir.empty()) {
     filename += g_conf.profiling_logger_subdir;
     ::mkdir( filename.c_str(), 0755 );   // make sure dir exists
     filename += "/";

diff --git a/src/common/ceph_argparse.cc b/src/common/ceph_argparse.cc
@@ -128,16 +128,6 @@ bool ceph_argparse_cmdline_val(void *field, int type, const char *val)
   return true;
 }
 
-void env_override(char **ceph_var, const char * const env_var)
-{
-  char *e = getenv(env_var);
-  if (!e)
-    return;
-  if (*ceph_var)
-    free(*ceph_var);
-  *ceph_var = strdup(e);
-}
-
 void env_to_vec(std::vector<const char*>& args)
 {
   char *p = getenv("CEPH_ARGS");

diff --git a/src/common/ceph_argparse.h b/src/common/ceph_argparse.h
@@ -82,7 +82,6 @@ class CephInitParameters
 };
 
 /////////////////////// Functions ///////////////////////
-extern void env_override(char **ceph_var, const char * const env_var);
 extern void env_to_vec(std::vector<const char*>& args);
 extern void env_to_deq(std::deque<const char*>& args);
 extern void argv_to_vec(int argc, const char **argv,

diff --git a/src/common/common_init.cc b/src/common/common_init.cc
@@ -33,25 +33,23 @@ int keyring_init(md_config_t *conf)
   if (!is_supported_auth(CEPH_AUTH_CEPHX))
     return 0;
 
-  const char *filename = conf->keyring;
-  string keyring_search = conf->keyring;
+  const char *filename = NULL;
   string new_keyring;
-  if (ceph_resolve_file_search(keyring_search, new_keyring)) {
+  if (ceph_resolve_file_search(conf->keyring, new_keyring)) {
     filename = new_keyring.c_str();
   }
 
   int ret = g_keyring.load(filename);
 
-  if (conf->key && conf->key[0]) {
-    string k = conf->key;
+  if (!conf->key.empty()) {
     EntityAuth ea;
-    ea.key.decode_base64(k);
+    ea.key.decode_base64(conf->key);
     g_keyring.add(*conf->name, ea);
 
     ret = 0;
-  } else if (conf->keyfile && conf->keyfile[0]) {
+  } else if (!conf->keyfile.empty()) {
     char buf[100];
-    int fd = ::open(conf->keyfile, O_RDONLY);
+    int fd = ::open(conf->keyfile.c_str(), O_RDONLY);
     if (fd < 0) {
       int err = errno;
       derr << "unable to open " << conf->keyfile << ": "
@@ -103,8 +101,8 @@ md_config_t *common_preinit(const CephInitParameters &iparams,
     case CODE_ENVIRONMENT_DAEMON:
       conf->daemonize = true;
       if (!(flags & CINIT_FLAG_UNPRIVILEGED_DAEMON_DEFAULTS)) {
-	conf->log_dir = strdup("/var/log/ceph");
-	conf->pid_file = strdup("/var/run/ceph/$type.$id.pid");
+	conf->log_dir = "/var/log/ceph";
+	conf->pid_file = "/var/run/ceph/$type.$id.pid";
       }
       conf->log_to_stderr = LOG_TO_STDERR_SOME;
       break;