Menu
▾
▴
[Freeorion-programmers] [ freeorion-Bugs-2929379 ] getBinDir() detects wrong directory
|
From: SourceForge.net <no...@so...> - 2010-01-11 10:21:59
|
Bugs item #2929379, was opened at 2010-01-10 19:02 Message generated for change (Comment added) made by olivleh1 You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=544942&aid=2929379&group_id=75752 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Oliver Lehmann (olivleh1) Assigned to: Nobody/Anonymous (nobody) Summary: getBinDir() detects wrong directory Initial Comment: Hi, with Revision 3131 of util/Directories.cpp some code was added to enhance the bin-dir lookup code. Unfortunally using argv0 has at least on FreeBSD the problem that when the binary is called from any directory by using the PATH environment variable to look the binary up, getBinDir() will return the directory the user is in when freeorion gets called. eg. I'm in /tmp and call "freeorion" through to PATH, /usr/local/bin/freeorion gets called. Unfortunally because of the added code, GetBinDir() returns /tmp. I suggest the attached patch to have a look into the determined directory if the "freeorion" binary exists there and if not continue with the "old" way... This makes it work here. ---------------------------------------------------------------------- >Comment By: Oliver Lehmann (olivleh1) Date: 2010-01-11 11:21 Message: unfortunally the latest patch does not work: util/Directories.cpp: In function 'void InitBinDir(const std::string&)': util/Directories.cpp:213: error: invalid conversion from 'const size_t*' to 'size_t*' util/Directories.cpp:213: error: initializing argument 4 of 'int sysctl(int*, u_int, void*, size_t*, void*, size_t)' By the way - NULL is defined because of the <cstdlib> include on FreeBSD where this code is only evaluated at all so having NULL there won't hurt imho and is a bit cleaner. What do you think? Please find the latest patch to make it work on FreeBSD. I've put back the sizeof() code to have it again working with sysctl(). I've made sure that not the fallback case was used. I changed the path there to /foo and printed the bin_path out. It returns /usr/local/bin. ---------------------------------------------------------------------- Comment By: Geoff Topping (geoffthemedio) Date: 2010-01-11 09:45 Message: Could you test the latest patch I attached? I switched the NULL 0 so, so that might cause something to break, but I'm also interested in if the rest of the changes broke anything in dependently of the NULL -> 0 change. And if it's working on FreeBSD, is it working by falling through to the default case where it looks in /usr/local/bin or is it finding the binary with the new code as intended? ---------------------------------------------------------------------- Comment By: Oliver Lehmann (olivleh1) Date: 2010-01-11 09:40 Message: one addition - __FreeBSD__ is always defined on FreeBSD ---------------------------------------------------------------------- Comment By: Oliver Lehmann (olivleh1) Date: 2010-01-11 09:40 Message: I changed the declaration of BUF_SIZE to size_t because on 64bit platforms such as mine, size_t is declared as long but gets a warning when comparing with "<0". should be read as: I changed the declaration of BUF_SIZE to size_t because on 64bit platforms such as mine, size_t is declared as long. So exe_path_size is declared as size_t but previously, BUF_SIZE was declared as int. Comparing long (size_t) with int results in a warning. + if (exe_path_size >= BUF_SIZE || exe_path_size < 0) + exe_path_size = BUF_SIZE - 1; // ensure buffer isn't accessed out of range Thats why I changed the declaration of BUF_SIZE to size_t. The patch I sent actually works on FreeBSD (verified that). The sysctl call has to stay like it is I guess (NULL, 0) regarding to it's manpage: To set a new value, newp is set to point to a buffer of length newlen from which the requested value is to be taken. If a new value is not to be set, newp should be set to NULL and newlen set to 0. http://www.freebsd.org/cgi/man.cgi?query=sysctl&sektion=3 Do you still want me something to test? ---------------------------------------------------------------------- Comment By: Geoff Topping (geoffthemedio) Date: 2010-01-11 09:36 Message: Does the patch as you just posted work for you on FreeBSD? Is __FreeBSD__ guaranteed to be declared on FreeBSD systems? Can 0 replace NULL (the latter which I don't think is always defined...?) Does the warning about comparing < 0 go away when declaring BUF_SIZE as size_t ? As far as I can tell, sizeof(buf) will alreays return BUF_SIZE, not the length of the string before the null character. Do you know if there's a way to get the actual text size? I've tried to work around this by initializing buf to all 0, and setting the last char in the array to 0 in all cases where the array will be used in the attached patch. Does it work? ---------------------------------------------------------------------- Comment By: Oliver Lehmann (olivleh1) Date: 2010-01-11 08:51 Message: If you do not fear ifdefs, this can be worked around to also add a FreeBSD case. I changed the declaration of BUF_SIZE to size_t because on 64bit platforms such as mine, size_t is declared as long but gets a warning when comparing with "<0". Please tell me your opinion. I'm open to anything. ---------------------------------------------------------------------- Comment By: Geoff Topping (geoffthemedio) Date: 2010-01-11 07:14 Message: This page has a few possible methods: http://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe I'd like to have something coded that will actually find the binary directory, rather than always defaulting to a hard-coded case. I'm not sure how to properly set things up so different code is compiled on FreeBSD than on Linux, though, so that might be difficult... ---------------------------------------------------------------------- Comment By: Oliver Lehmann (olivleh1) Date: 2010-01-11 07:06 Message: Hi, unfortunally, /proc/self/exe is not POSIX conform so it is bound to Linux only. FreeBSD does not have it so I cannot test this part of the patch :( I'll see if I find something out... ---------------------------------------------------------------------- Comment By: Geoff Topping (geoffthemedio) Date: 2010-01-11 00:05 Message: I've done some more research and found a possible solution using the "readlink" function. I can't test this myself on linux or freebsd, so could you / someone try it out, fix any fixable errors, and report back? ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=544942&aid=2929379&group_id=75752 |