Re: [PATCH] Fix testRock on FreeBSD

On 09/19/2012 10:38 AM, Kinkie wrote:

> I've done some testing on FreeBSD 9 (which fails the unit tests in testRock).
> Apparently the issue is caused by shm_open in the ipc channels, which
> fails with EINVAL because the shm path is not absolute.
> This patch changes the path for the unit test shm to /tmp; it is
> confirmed working on FreeBSD-9; couldn't be tested on OpenSuSE yet.

Hi Kinkie,

    Thank you for working on this. The test case is using the current
directory for the cache files (TESTDIR). It would be better to keep
using the current directory for shared memory files as well, to reduce
the number of different directories we need to worry about (each of them
could lead to troubles in one environment or another). This can be
accomplished while still satisfying the "absolute path" restriction by
getting the current directory name using getcwd(3), right?

However, I cannot be against the proposed simple change since it fixes
one build while there is no substantial evidence that it may break
others. Your call wither using getcwd(3) is better than "/tmp".

> === modified file 'src/tests/testRock.cc'
> --- src/tests/testRock.cc 2012-08-31 16:57:39 +0000
> +++ src/tests/testRock.cc 2012-09-19 13:26:45 +0000
> @@ -45,7 +45,7 @@
> throw std::runtime_error("Failed to clean test work directory");
>
> // use current directory for shared segments (on path-based OSes)
> - Ipc::Mem::Segment::BasePath = ".";
> + Ipc::Mem::Segment::BasePath = "/tmp";
>
> Store::Root(new StoreController);

You will need to update the comment if this directory change is committed.

BTW, what happens if two concurrent "make check" sessions try to use
/tmp for this test case? Will one of them occasionally fail due to
filename collisions? I think the probability of that collision happening
is very low, even in automated build environments, but you might want to
add a comment about it.

+0

Thank you,

Alex.
Received on Thu Sep 20 2012 - 21:46:27 MDT