On 29.12.2014 13:59, Stefan Fuhrmann wrote: > Hi there, > > FSX code contains various violations to our naming rules, > mostly taken over FSFS. I thought about a scheme that > complies to our rules but also refines them. > > I'd like to amend our coding guideline with the following > suggestions (not as a strict requirement). The first one is > actually "new" while the other two have already been used > by various portions of our fs_* libs: > > 1. Use the svn_ prefix only for identifiers meant to be used > by other libs, i.e. only for declarations in the ./include sub- > tree. We currently lack that distinction and it lead to minor > confusion in the past.
If it's a non-static identifier (i.e., if it can possibly be exported from some library), then we have to retain the svn_ prefix in order to reduce the chance of name collisions in downstream uses. > 2. If the library name contains multiple elements, use only > the last one as prefix for internal identifiers. In combination > with the svn_ prefix, use the full library name. > > 3. Static functions implementing a v-table function should be > named <lib>_<func>. <func> is the name of the respective > API function without its prefixes. > > Combined with the existing rules, this is how a "commit" > function in libsvn_fs_base would be named. This is a theoretical > example; not all of these identifiers actually do exist in SVN: > > * commit - static function > * base_commit - static function implementing svn_fs_commit This is almost what we're already doing, although I have trouble visualising a connection between "base" and "svn_fs". It would be far more obvious to use the name "fs_base_commit", which brings us back to our current naming convention. I suspect you're trying invent a way to use shorter identifiers. While that's a commendable goal, it's never a good idea to do that at the expense of clarity. > * base__commit - library-local, defined in some local header There's no such thing as library-local symbols; see above. In general, you cannot expect to be able to hide such symbols from external library users. Yes, there are tricks you can use in some object formats, but we don't rely on them because they're rarely portable (in the semantic sense) between formats and platforms. I'd be overjoyed if someone could demonstrate that we can create truly library-local symbol names that cannot be accidentally exported, and that we can consistently do this on all currently supported and future platforms. However, I suspect this may be a futile goal. > * svn_fs_base__commit - to be used by any SVN lib and must > be declared in ./include/private > * svn_fs_base_commit - to be used by any part of an application, > subject to compatibility rules and must be declared in ./include IIUC another thing you're trying to solve is the ... call it "ideological" difference between private symbols declared in ./include/private and such symbols declared in library-local headers. Our current naming convention for both is svn_libname__symbol, which I agree may be confusing. But ... as I said, I do not agree that we can safely drop the svn_ prefix from so-called "library-local" symbols. An obvious alternative would be svn__fs_base_commit vs. svn_fs_base__commit but that's even more confusing; as is svn__fs_base__commit. I suspect the best solution for this case would be to invent a completely new prefix; consider, e.g., xvn__fs_base_commit; which is different from the current private API naming convention. I'm not sure what to do here, especially as our "library-private" naming is already less than consistent. Inventing a new convention for these names implies that, at some point, we'd have to rename them all ... which would involve a lot of code churn for dubious benefit. -- Brane