On 7 February 2015 at 20:05, Dirk Hohndel <[email protected]> wrote: > On Sat, Feb 07, 2015 at 06:26:38PM +0100, Anton Lundin wrote: >> On 06 February, 2015 - Lubomir I. Ivanov wrote: >> > >> > in terms of our current scheme for updating ssrf-version.h, with each >> > git HEAD change one potential small issue is present where the macros >> > from the file itself are used on compile time by a number of files. >> > this forces recompilation of said files into object code even if they >> > effectively search for the same literals in the pool of the >> > executable. >> > >> > to solve the issue, i suggest the introduction of a version.c / >> > version.h pair which is a simple abstraction layer on top of the >> > generated ssrf-version.h. (more files, i know...) >> > >> > by changing the git HEAD, version.c will be the only object code that >> > has to be recompiled and the file itself will hold a simple API in the >> > lines of: >> > >> > const char *get_version(); >> > const char *get_version_git(); >> > const char *get_version_canonical(); >> > >> > it will be exposed via version.h and such that all the affected files >> > can use instead of the macros. the macros will be still available via >> > ssrf-version.h, but should not be used directly, especially in >> > complicated C++/Qt files. >> > >> > what this will do is to ensure that a version string is only retrieved >> > via a controlled branch (i.e. one of the 3 functions above). such >> > branches will be the only direct access points to the literal pool and >> > thus the recompilation process will be simplified. >> > >> > will such a patch be accepted? comments? > > Yes, I'd be fine with such a patch. > >> I've thought about attacking the same problem myself, so I'm really >> happy that someone else have started to think about it to. >> >> My thought was just a: >> extern const char *version; >> extern const char *git_version; >> extern const char *canonical_version; >> >> Any up/down-sides towards using a extern or a function? >
as long as these are in version.h and not dive.h i don't see much of a problem. a couple more comments bellow. > Interesting question. I'm sure the standard and language gurus will have > an opinion on that. I don't think it matters, but that may just show my > ignorance. :-) > We use both approaches already for other global data (dive_table or > system_default_filename()), so I don't really care. I guess using a > variable would be less potential overhead (without marking the function as > inline the compiler likely will generate a function call and a stack frame > and all that) - but seriously, that's a completely bogus argument. The > layers and layers and layers of code that Qt adds to everything we do... > yep, the jump and stack frame overhead is non-existing for a modern application and hardware as we won't be calling it 10e+6 times per second. but like you said there is indeed small overhead unless it's inlined, but nothing prevents us from adding the "inline" keyword. one *real* benefit of using functions for everything is that even for a small thing like version retrieval, it's wrapped in a function the contents of which can be modified any time without breaking the API. > So I'll take a patch that uses either approach. which approach should we pick? > > Also, Lubomir, I have asked Thiago to look at the qmake patch you sent and > he promised to do so this weekend. I'm waiting to hear his feedback before > pulling that (I assume this patch here would depend on that other patch?) > ok. actually, if ssrf-version.h is generated by the Makefile in one way or another the two patches can be applied without succession. lubomir -- _______________________________________________ subsurface mailing list [email protected] http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface
