In my API library (C++) I am wrapping a shell process (spawned using popen) with several classes to manipulate a file system. The base class ProcessFileWrap wraps general filesystem tasks and executes other custom tasks from its derived classes. It looks like below in pseudocode:// ProcessFileWrap base class methodsExecCustomProcess(string process) // does popen(process, r)ChangeDir(string path) // changes the current directoryAddFolder(string path) // wrapper for mkdirAddFile(string path) // wrapper for touchMakeLink(string pathToFile, string pathToLink) // wrapper for ln -s// ... And so onThere are several derived classes. One of them is GitFileWrap, which wraps git processes like so (again in psuedocode). It needs to do some specific file system tasks after or during the process:// GitFileWrap derived class methods// Calls ChangeDir(pathToRepo) and ExecCustomProcess(git pull)// Afterwards it creates links for each pulled file in a custom // directory via MakeLink(file, customLoc).Pull()// Calls ExecCustomProcess(git commit -m obj.GetCommitMessage())// A GitObject represents a git versioned object in the filesystem.Commit(GitObject obj)// ... And so onSo users of the API would call methods from the derived classes as needed.I am aware that wrapping processes is in general not best practice, so if you can, please comment on the design with the assumption that there isn't really another alternative (or I guess you could, but I would appreciate comments on the design above). Thanks!Note: It is not possible for me to use libgit2
Wrapping a shell process to manipulate a file system, how to improve design?
design;c++;object oriented design;api design
Don't use external processes if you can help it. Use the system calls directly: chdir(path), mkdir(pathname, mode), std::fstream or open(path, O_CREAT) instead of touch, symlink(target, path) instead of ln -s. In a C++ program, there are just no good reasons to use the wrapper binaries instead of just calling these syscalls directly.When you really have to use external commands, don't use popen(). popen() is slow and hopelessly insecure because it always starts a shell and therefore it also needlessly interprets globs and shell variables. The safe way to start a subprocess is with fork() and execve() or posix_spawn(). These functions passes the program files and parameters as an array instead of requiring you to interpolate and escape your arguments into a strings, only to have a shell interpreter try to parse the command arguments to figure out how to split your strings that it eventually passes to execve anyway.