diff --git a/main.cpp b/main.cpp index c72a394f5c70412e090388aa5a409891f2d0be4e..4d281ba156e580952ce16f739fb05cbbd05f0682 100644 --- a/main.cpp +++ b/main.cpp @@ -9,7 +9,7 @@ using namespace miosix; int main() { //iprintf("Hello world, write your application here\n"); - DevFs *dfs=new DevFs; + intrusive_ref_ptr<DevFs> dfs(new DevFs); FilesystemManager& fsm=FilesystemManager::instance(); fsm.kmount("/dev",dfs); FileDescriptorTable fdt; diff --git a/miosix/filesystem/devfs/devfs.cpp b/miosix/filesystem/devfs/devfs.cpp index 0170eae0fd6fd3996767e0136776275249561637..bf634d61357204e72e977c5c7a4f9f207b1e6375 100644 --- a/miosix/filesystem/devfs/devfs.cpp +++ b/miosix/filesystem/devfs/devfs.cpp @@ -37,39 +37,39 @@ namespace miosix { DevFs::DevFs() { - files["null"]=intrusive_ref_ptr<FileBase>(new NullFile(this)); - files["zero"]=intrusive_ref_ptr<FileBase>(new ZeroFile(this)); + files[StringPart("null")]=intrusive_ref_ptr<FileBase>(new NullFile); + files[StringPart("zero")]=intrusive_ref_ptr<FileBase>(new ZeroFile); } int DevFs::open(intrusive_ref_ptr<FileBase>& file, StringPart& name, int flags, int mode) { - return -1; //FIXME + //TODO: mode & flags + Lock<Mutex> l(mutex); + map<StringPart,intrusive_ref_ptr<FileBase> >::iterator it=files.find(name); + if(it==files.end()) return -ENOENT; + file=it->second; + return 0; } int DevFs::lstat(StringPart& name, struct stat *pstat) { - return -1; //FIXME -} - -int DevFs::readlink(StringPart& name, std::string& target) -{ - return -1; //FIXME + Lock<Mutex> l(mutex); + map<StringPart,intrusive_ref_ptr<FileBase> >::iterator it=files.find(name); + if(it==files.end()) return -ENOENT; + it->second->fstat(pstat); + return 0; } -bool DevFs::supportsSymlinks() const { return false; } - bool DevFs::areAllFilesClosed() { - //Lock the mutex just in case we'll want to support dynamically removing - //device files at runtime Lock<Mutex> l(mutex); //Can't use openFileCount in devFS, as one instance of each file is stored //in the map. Rather, check the reference count value. No need to use //atomic ops to make a copy of the file before calling use_count() as the //existence of at least one reference in the map guarantees the file won't //be deleted. - map<string,intrusive_ref_ptr<FileBase> >::iterator it; + map<StringPart,intrusive_ref_ptr<FileBase> >::iterator it; for(it=files.begin();it!=files.end();++it) if(it->second.use_count()>1) return false; return true; diff --git a/miosix/filesystem/devfs/devfs.h b/miosix/filesystem/devfs/devfs.h index 50d7b58ffbace69cac2ab00c3a1275791a1cc290..bc5a225a4aff0f0a479631f3f33a37a5a223311b 100644 --- a/miosix/filesystem/devfs/devfs.h +++ b/miosix/filesystem/devfs/devfs.h @@ -35,6 +35,31 @@ namespace miosix { +/** + * DevFs is a special filesystem meant to access devices as they were files. + * For this reason, it is a little different from other filesystems. Normal + * filesystems create FileBase objects ondemand, to answer an open() call. Such + * files have a parent pointer that points to the filesystem. On the contrary, + * DevFs is a collection of device files. Each device file is a different + * subclass of FileBase that overrides some of its member functions to access + * the handled device. These FileBase subclasses do not have a parent pointer + * into DevFs, instead, DevFs holds a map of device names and pointers into + * these derived FileBase objects, which always exist, even if they are not + * opened anywere. Opening the same file name multiple times returns the same + * instance of the file object. + * + * The reason why device files can't have the parent pointer that points into + * DevFs is that DevFs already has a pointer into those files, and since they're + * both refcounted pointers that would be a cycle of refcounted pointer that + * would cause problem if DevFs is ever umounted. + * + * Also a note on the behaviour when umounting DevFs: since the files don't + * have a parent pointer into DevFs umounting DevFs will immediately delete + * the filesystem, while the individual files won't be deleted until the + * processes that have them opened close them. This isn't an unreasonable + * behaviour, and also it must be considered that umounting DevFs shouldn't + * happen in most practical cases. + */ class DevFs : public FilesystemBase { public: @@ -64,22 +89,6 @@ public: */ virtual int lstat(StringPart& name, struct stat *pstat); - /** - * Follows a symbolic link - * \param path path identifying a symlink, relative to the local filesystem - * \param target the link target is returned here if the call succeeds. - * Note that the returned path is not relative to this filesystem, and can - * be either relative or absolute. - * \return 0 on success, a negative number on failure - */ - virtual int readlink(StringPart& name, std::string& target); - - /** - * \return true if the filesystem supports symbolic links. - * In this case, the filesystem should override readlink - */ - virtual bool supportsSymlinks() const; - /** * \internal * \return true if all files belonging to this filesystem are closed @@ -88,7 +97,7 @@ public: private: Mutex mutex; - std::map<std::string,intrusive_ref_ptr<FileBase> > files; + std::map<StringPart,intrusive_ref_ptr<FileBase> > files; }; } //namespace miosix diff --git a/miosix/filesystem/file.cpp b/miosix/filesystem/file.cpp index cb136f16178b050daa47a6bc2c199ca00f5dc751..67473a2b400ff9fa1a115c2e820f6ef8d8b8f1a0 100644 --- a/miosix/filesystem/file.cpp +++ b/miosix/filesystem/file.cpp @@ -129,14 +129,9 @@ int ZeroFile::sync() // class TerminalDevice // -//TODO FileBase(0): should we override getPartent() to return device->getParent()? TerminalDevice::TerminalDevice(intrusive_ref_ptr<FileBase> device) - : FileBase(0), device(device), echo(false), binary(false), skipNewline(false) {} - -void TerminalDevice::changeDevice(intrusive_ref_ptr<FileBase> newDevice) -{ - device=newDevice; -} + : FileBase(device->getParent()), device(device), echo(false), + binary(false), skipNewline(false) {} ssize_t TerminalDevice::write(const void *data, size_t len) { diff --git a/miosix/filesystem/file.h b/miosix/filesystem/file.h index 945367205c07f9b247e31c300f6daefe699c29dd..ec195c89cd18e6023e3288147c535c8813129e1c 100644 --- a/miosix/filesystem/file.h +++ b/miosix/filesystem/file.h @@ -51,7 +51,7 @@ public: * Constructor * \param parent the filesystem to which this file belongs */ - FileBase(FilesystemBase *parent) : parent(parent) {} + FileBase(intrusive_ref_ptr<FilesystemBase> parent) : parent(parent) {} /** * Write data to the file, if the file supports writing. @@ -104,7 +104,7 @@ public: /** * \return a pointer to the parent filesystem */ - const FilesystemBase *getParent() const { return parent; } + const intrusive_ref_ptr<FilesystemBase> getParent() const { return parent; } /** * File destructor @@ -115,7 +115,7 @@ private: FileBase(const FileBase&); FileBase& operator=(const FileBase&); - FilesystemBase *parent; ///< All files must have a parent filesystem + intrusive_ref_ptr<FilesystemBase> parent; ///< Files may have a parent fs }; /** @@ -128,7 +128,7 @@ public: * Constructor * \param parent the filesystem to which this file belongs */ - NullFile(FilesystemBase *parent) : FileBase(parent) {} + NullFile() : FileBase(intrusive_ref_ptr<FilesystemBase>()) {} /** * Write data to the file, if the file supports writing. @@ -189,7 +189,7 @@ public: * Constructor * \param parent the filesystem to which this file belongs */ - ZeroFile(FilesystemBase *parent) : FileBase(parent) {} + ZeroFile() : FileBase(intrusive_ref_ptr<FilesystemBase>()) {} /** * Write data to the file, if the file supports writing. @@ -253,12 +253,6 @@ public: */ TerminalDevice(intrusive_ref_ptr<FileBase> device); - /** - * Change the proxed object - * \param newDevice new proxed device - */ - void changeDevice(intrusive_ref_ptr<FileBase> newDevice); - /** * Convenience function to write a text string, terminated with \0. * \param str the string to write. diff --git a/miosix/filesystem/file_access.cpp b/miosix/filesystem/file_access.cpp index 7b6820b7b0f0a0de587d48f1c26c5f699f5a68d3..79f9bcaefa7214a47a1f852e75b9ccef39e809d4 100644 --- a/miosix/filesystem/file_access.cpp +++ b/miosix/filesystem/file_access.cpp @@ -57,11 +57,11 @@ public: * \param off offset into path where the subpath relative to the current * filesystem starts */ - ResolvedPath(FilesystemBase *fs, int offset) + ResolvedPath(intrusive_ref_ptr<FilesystemBase> fs, int offset) : result(0), fs(fs), off(offset) {} - int result; ///< 0 on success, a negative number on failure - FilesystemBase *fs; ///< pointer to the filesystem to which the file belongs + int result; ///< 0 on success, a negative number on failure + intrusive_ref_ptr<FilesystemBase> fs; ///< pointer to the filesystem to which the file belongs /// path.c_str()+off is a string containing the relative path into the /// filesystem for the looked up file int off; @@ -76,6 +76,8 @@ int FilesystemBase::readlink(StringPart& name, string& target) return -EINVAL; //Default implementation, for filesystems without symlinks } +bool FilesystemBase::supportsSymlinks() const { return false; } + bool FilesystemBase::areAllFilesClosed() { return openFileCount==0; } void FilesystemBase::fileCloseHook() { atomicAdd(&openFileCount,-1); } @@ -308,7 +310,7 @@ public: * Constructor * \param fs map of all mounted filesystems */ - PathResolution(const map<StringPart,FilesystemBase*>& fs) + PathResolution(const map<StringPart,intrusive_ref_ptr<FilesystemBase> >& fs) : filesystems(fs) {} /** @@ -362,13 +364,13 @@ private: bool checkLastComponent(string& path); /// Mounted filesystems - const map<StringPart,FilesystemBase*>& filesystems; + const map<StringPart,intrusive_ref_ptr<FilesystemBase> >& filesystems; /// Pointer to root filesystem - FilesystemBase *root; + intrusive_ref_ptr<FilesystemBase> root; /// Current filesystem while looking up path - FilesystemBase *fs; + intrusive_ref_ptr<FilesystemBase> fs; /// True if current filesystem supports symlinks bool syms; @@ -393,7 +395,7 @@ private: ResolvedPath PathResolution::resolvePath(string& path, bool followLastSymlink) { char rootPath[2]; rootPath[0]='/'; rootPath[1]='\0';//A non-const "/" string - map<StringPart,FilesystemBase*>::const_iterator it; + map<StringPart,intrusive_ref_ptr<FilesystemBase> >::const_iterator it; it=filesystems.find(StringPart(rootPath)); if(it==filesystems.end()) return ResolvedPath(-ENOENT); //should not happen root=fs=it->second; @@ -458,7 +460,7 @@ int PathResolution::normalPathComponent(string& path, bool followIfSymlink) //NOTE: index<path.length() is necessary as for example /dev and // /dev/ should resolve to the directory in the root filesystem, not //to the root directory of the /dev filesystem - map<StringPart,FilesystemBase*>::const_iterator it; + map<StringPart,intrusive_ref_ptr<FilesystemBase> >::const_iterator it; it=filesystems.find(StringPart(path,index-1)); if(it!=filesystems.end()) { @@ -550,7 +552,7 @@ int PathResolution::recursiveFindFs(string& path) indexIntoFs=0; break; } - map<StringPart,FilesystemBase*>::const_iterator it; + map<StringPart,intrusive_ref_ptr<FilesystemBase> >::const_iterator it; it=filesystems.find(StringPart(path,backIndex)); if(it!=filesystems.end()) { @@ -582,7 +584,7 @@ FilesystemManager& FilesystemManager::instance() return instance; } -int FilesystemManager::kmount(const char* path, FilesystemBase* fs) +int FilesystemManager::kmount(const char* path, intrusive_ref_ptr<FilesystemBase> fs) { if(path==0 || path[0]=='\0' || fs==0) return -EFAULT; Lock<Mutex> l(mutex); @@ -592,28 +594,29 @@ int FilesystemManager::kmount(const char* path, FilesystemBase* fs) if(len>PATH_MAX) return -ENAMETOOLONG; string temp(path); if(filesystems.insert(make_pair(StringPart(temp),fs)).second==false) - return -EBUSY; + return -EBUSY; //Means already mounted else return 0; } -int FilesystemManager::umount(const char* path) +int FilesystemManager::umount(const char* path, bool force) { + typedef + typename map<StringPart,intrusive_ref_ptr<FilesystemBase> >::iterator fsIt; + if(path==0 || path[0]=='\0') return -ENOENT; int len=strlen(path); if(len>PATH_MAX) return -ENAMETOOLONG; string pathStr(path); Lock<Mutex> l(mutex); //A reader-writer lock would be better - map<StringPart,FilesystemBase*>::iterator it; - it=filesystems.find(StringPart(pathStr)); + fsIt it=filesystems.find(StringPart(pathStr)); if(it==filesystems.end()) return -EINVAL; //This finds all the filesystems that have to be recursively umounted //to umount the required filesystem. For example, if /path and /path/path2 //are filesystems, umounting /path must umount also /path/path2 - vector<map<StringPart,FilesystemBase*>::iterator> fsToUmount; - map<StringPart,FilesystemBase*>::iterator it2; - for(it2=filesystems.begin();it2!=filesystems.end();++it2) + vector<fsIt> fsToUmount; + for(fsIt it2=filesystems.begin();it2!=filesystems.end();++it2) if(it2->first.startsWith(it->first)==0) fsToUmount.push_back(it2); //Now look into all file descriptor tables if there are open files in the @@ -630,11 +633,12 @@ int FilesystemManager::umount(const char* path) { intrusive_ref_ptr<FileBase> file=(*it3)->getFile(i); if(!file) continue; - vector<map<StringPart,FilesystemBase*>::iterator>::iterator it4; + vector<fsIt>::iterator it4; for(it4=fsToUmount.begin();it4!=fsToUmount.end();++it4) { if(file->getParent()!=(*it4)->second) continue; - return -EBUSY; + if(force==false) return -EBUSY; + (*it3)->close(i); //If forced umount, close the file } } } @@ -643,20 +647,30 @@ int FilesystemManager::umount(const char* path) //but check if it is really so, as there is a possible race condition //which is the read/close,umount where one thread performs a read (or write) //operation on a file descriptor, it gets preempted and another thread does - //a close on that descriptor and an umount of the filesystem. - //In such a case there is no entry in the descriptor table (as close was - //called) but the operation is still ongoing. - vector<map<StringPart,FilesystemBase*>::iterator>::iterator it5; - for(it5=fsToUmount.begin();it5!=fsToUmount.end();++it5) - if((*it5)->second->areAllFilesClosed()==false) return -EBUSY; + //a close on that descriptor and an umount of the filesystem. Also, this may + //happen in case of a forced umount. In such a case there is no entry in + //the descriptor table (as close was called) but the operation is still + //ongoing. + vector<fsIt>::iterator it5; + const int maxRetry=3; //Retry up to three times + for(int i=0;i<maxRetry;i++) + { + bool failed=false; + for(it5=fsToUmount.begin();it5!=fsToUmount.end();++it5) + { + if((*it5)->second->areAllFilesClosed()) continue; + if(force==false) return -EBUSY; + failed=true; + break; + } + if(!failed) break; + if(i==maxRetry-1) return -EBUSY; //Failed to umount even if forced + Thread::sleep(1000); //Wait to see if the filesystem operation completes + } //It is now safe to umount all filesystems for(it5=fsToUmount.begin();it5!=fsToUmount.end();++it5) - { - FilesystemBase *toUmount=(*it5)->second; filesystems.erase(*it5); - delete toUmount; - } return 0; } diff --git a/miosix/filesystem/file_access.h b/miosix/filesystem/file_access.h index c3b51a97bddceb308e22b343e38a4c7eaddb3b0c..2a4f7878c45ca68d79921e8a27a999f31a752b21 100644 --- a/miosix/filesystem/file_access.h +++ b/miosix/filesystem/file_access.h @@ -49,9 +49,11 @@ class ResolvedPath; class StringPart; /** - * All filesystems derive from this class + * All filesystems derive from this class. Classes of this type are reference + * counted, must be allocated on the heap and managed through + * intrusive_ref_ptr<FilesystemBase> */ -class FilesystemBase +class FilesystemBase : public IntrusiveRefCounted { public: /** @@ -94,7 +96,7 @@ public: * \return true if the filesystem supports symbolic links. * In this case, the filesystem should override readlink */ - virtual bool supportsSymlinks() const=0; + virtual bool supportsSymlinks() const; /** * \internal @@ -503,14 +505,15 @@ public: * to the FilesystemManager class * \return 0 on success, a negative number on failure */ - int kmount(const char *path, FilesystemBase *fs); + int kmount(const char *path, intrusive_ref_ptr<FilesystemBase> fs); /** * Unmounts a filesystem * \param path path to a filesytem + * \param force true to umount the filesystem even if busy * \return 0 on success, or a negative number on error */ - int umount(const char *path); + int umount(const char *path, bool force=false); /** * Resolve a path to identify the filesystem it belongs @@ -562,7 +565,10 @@ private: FilesystemManager& operator=(const FilesystemManager&); Mutex mutex; ///< To protect against concurrent access - std::map<StringPart,FilesystemBase*> filesystems; ///< Mounted filesystem + + /// Mounted filesystem + std::map<StringPart,intrusive_ref_ptr<FilesystemBase> > filesystems; + // #ifdef WITH_PROCESSES std::list<FileDescriptorTable*> fileTables; ///< Process file tables // #else //WITH_PROCESSES