Re: [PATCH] Fix testRock on FreeBSD

From: Kinkie <gkinkie_at_gmail.com>
Date: Fri, 21 Sep 2012 10:43:28 +0200

On Thu, Sep 20, 2012 at 11:46 PM, Alex Rousskov
<rousskov_at_measurement-factory.com> wrote:
> 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".

> 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.

Hi Alex,
  The best solution at all to avoid filename collisions would be to
use a guaranteed-unique directory (e.g. using mktemp(3)), or to rely
on something guaranteed unique (e.g. the pid).

I've changed to use getcwd(3). Tested OK on Ubuntu.

=== modified file 'src/tests/testRock.cc'
--- src/tests/testRock.cc 2012-09-19 15:25:23 +0000
+++ src/tests/testRock.cc 2012-09-21 08:00:11 +0000
@@ -21,6 +21,8 @@
 #if HAVE_STDEXCEPT
 #include <stdexcept>
 #endif
+#include <unistd.h>
+

 #define TESTDIR "testRock__testRockSearch"

@@ -28,6 +30,8 @@

 extern REMOVALPOLICYCREATE createRemovalPolicy_lru;

+static char cwd[MAXPATHLEN];
+
 static void
 addSwapDir(testRock::SwapDirPointer aStore)
 {
@@ -45,7 +49,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 = "/tmp";
+ Ipc::Mem::Segment::BasePath = getcwd(cwd,MAXPATHLEN);

     Store::Root(new StoreController);

-- 
    /kinkie
Received on Fri Sep 21 2012 - 08:43:40 MDT

This archive was generated by hypermail 2.2.0 : Sat Sep 22 2012 - 12:00:07 MDT