Bug 93464

Hello,

Please review the patch and let us know the response.

Thanks
Amit Tewari

I applied the patch to dbus master, compiled with cmake on linux (which worked out of the box) and did run the server with

bin/test-multithread-server &
bin/test-multithread-client 

and got the line 

Client : Reply received  thread = ...

125 times. 

Is that the expected result ? 

In that case other test print out a dedicated test status 

...:~/src/dbus-cmake-build> bin/test-refs
/refs/connection: OK
/refs/message: OK
/refs/pending-call: OK
/refs/server: OK

(In reply to Ralf Habacker from comment #2)
> I applied the patch to dbus master, compiled with cmake on linux (which
> worked out of the box) and did run the server with
> bin/test-multithread-server &
bin/test-multithread-client 

and got the line
> Client : Reply received  thread = ...

125 times. 

Is that the expected
> result ? 

In that case other test print out a dedicated test status 
> ...:~/src/dbus-cmake-build> bin/test-refs
/refs/connection: OK
> /refs/message: OK
/refs/pending-call: OK
/refs/server: OK


Yes actually we are running 5 thread that runs in loop_count=25 to send message to bus and verify the Id in reply received. since we received all reply correctly so we got message and its success case.

Hello 

Please let us know your response. 

Thanks

(In reply to amit tewari from comment #3)

> Yes actually we are running 5 thread that runs in loop_count=25 to send
> message to bus and verify the Id in reply received. since we received all
> reply correctly so we got message and its success case.

This test should be used to test the recently compiled dbus source ? 
In that case some setup required:
1. start a local dbus-damon 
2. start the server providing the interface to test
3. run the client
4. after test exits terminate server and local dbus-daemin 

This setup is normally done by the dbus test suite, to which this test should be added to be able to be runable by make check. Your patch is missing this integration.

(In reply to Ralf Habacker from comment #5)
> (In reply to amit tewari from comment #3)

> Yes actually we are running 5
> thread that runs in loop_count=25 to send
> message to bus and verify the Id
> in reply received. since we received all
> reply correctly so we got message
> and its success case.

This test should be used to test the recently
> compiled dbus source ? 
In that case some setup required:
1. start a local
> dbus-damon 
2. start the server providing the interface to test
3. run the
> client
4. after test exits terminate server and local dbus-daemin 

This
> setup is normally done by the dbus test suite, to which this test should be
> added to be able to be runable by make check. Your patch is missing this
> integration.

I actually tested my testcase integrating into the dbus test/name-test/run-test.sh  script but since it can run only maximum 8 test case and also it can run only the testcases currently added in the run-test.sh script due to which I am not able to add it into dbus testsuite.
 Can you please add this to the name-test/run-test.sh script and then it will do the setup automatically.

Comment on attachment 120621 [details] [review]
[Patch] New multi-threaded message sending test case.

Review of attachment 120621 [details] [review]:
-----------------------------------------------------------------

compiling on dbus master with autotools on linux fails

The related build has been configured with --enable-tests

As already mentioned the integration in test/name-test/run-test.sh is missing. 

The number of tests could be increased in run-test.sh at the following fragment
... 
# TAP test plan: we will run 8 tests
echo "1..8"

Please fix the remaining issue.

::: test/name-test/test-multithread-client.c
@@ +18,5 @@
> +  exit (1);
> +}
> +
> +void*
> +send_multithread_data()

error: no previous prototype for ‘send_multithread_data’ [-Werror=missing-prototypes]

@@ +59,5 @@
> +      die("Client: Error getting ARG from Reply");
> +
> +    if(!(i == Ret_Id && ack && !strcmp(ack,"DataRecieved")))
> +      die ("Invalid reply from server");
> +    printf("Client : Reply received  thread = %u\n", pthread_self());

: no previous prototype for ‘send_multithread_data’ [-Werror=missing-prototypes]

@@ +75,5 @@
> +main(int argc, char** argv)
> +{
> +  int i = 0;
> +  dbus_threads_init_default();
> +  pthread_t mythread[MAXTHREAD];

error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]

(In reply to Ralf Habacker from comment #7)
> Comment on attachment 120621 [details] [review] [review]
[Patch] New multi-threaded
> message sending test case.

Review of attachment 120621 [details] [review] [review]:
> -----------------------------------------------------------------

compiling
> on dbus master with autotools on linux fails

The related build has been
> configured with --enable-tests

As already mentioned the integration in
> test/name-test/run-test.sh is missing. 

The number of tests could be
> increased in run-test.sh at the following fragment
... 
# TAP test plan: we
> will run 8 tests
echo "1..8"

Please fix the remaining issue.

:::
> test/name-test/test-multithread-client.c
@@ +18,5 @@
> +  exit (1);
> +}
> +
> > +void*
> +send_multithread_data()

error: no previous prototype for
> ‘send_multithread_data’ [-Werror=missing-prototypes]

@@ +59,5 @@
> +     
> die("Client: Error getting ARG from Reply");
> +
> +    if(!(i == Ret_Id &&
> ack && !strcmp(ack,"DataRecieved")))
> +      die ("Invalid reply from
> server");
> +    printf("Client : Reply received  thread = %u\n",
> pthread_self());

: no previous prototype for ‘send_multithread_data’
> [-Werror=missing-prototypes]

@@ +75,5 @@
> +main(int argc, char** argv)
>
> +{
> +  int i = 0;
> +  dbus_threads_init_default();
> +  pthread_t
> mythread[MAXTHREAD];

error: ISO C90 forbids mixed declarations and code
> [-Werror=declaration-after-statement]


Can you please let me know how you compile the code to reproduce these errors. I will be following the same to rectify them.

I do add these test cases to run-test.sh in the end script but still it say some errors , 
" ERROR: run-test.sh 9 test-multithread-client # UNPLANNED
ERROR: run-test.sh - too many tests run (expected 8, got 9
"
I added testcase in the end in run-test.sh script
============

# TAP test plan: we will run 8 tests
echo "1..8"

c_test test-ids
c_test test-pending-call-dispatch
c_test test-pending-call-timeout
c_test test-threads-init
c_test test-privserver-client
c_test test-shutdown
py_test test-activation-forking.py
c_test test-autolaunch
c_test test-multithread-client
                                          
======================

I can see name-test testcases run with "--enable-embedded-tests" in configure.

Created attachment 121435 [details] [review]
Updated patch with removed compile time warnings

Updated patch with removed compile time warnings

Created attachment 121438 [details] [review]
[Patch] Updated patch with compilte time warning removed with latest master branch

This Patch is handling all compile time error and is tested with latest master branch

(In reply to amit tewari from comment #8)

> Can you please let me know how you compile the code to reproduce these
> errors. I will be following the same to rectify them.

1. checkout dbus-source from git master branch into <dbus-source-dir>
2. create <dbus-build-dir> and enter it 
3. run <dbus-source-dir>/autogen.sh --enable-tests
4. run make

> 
> I do add these test cases to run-test.sh in the end script but still it say
> some errors , 
> " ERROR: run-test.sh 9 test-multithread-client # UNPLANNED
> ERROR: run-test.sh - too many tests run (expected 8, got 9
> "
> I added testcase in the end in run-test.sh script
> ============
> 
> # TAP test plan: we will run 8 tests
> echo "1..8"

# TAP test plan: we will run 9 tests
echo "1..9"

I have updated the patch for all compile time errors.
Can you please test at your end integrating in run-test.sh as I am getting still getting the errors.

(In reply to amit tewari from comment #12)
> I have updated the patch for all compile time errors.
> Can you please test at your end integrating in run-test.sh as I am getting
> still getting the errors.

With the following patch 

diff --git a/test/name-test/run-test.sh b/test/name-test/run-test.sh
index 8b9e7ad..8aaef68 100755
--- a/test/name-test/run-test.sh
+++ b/test/name-test/run-test.sh
@@ -67,8 +67,8 @@ py_test () {
 }
 
 test_num=1
-# TAP test plan: we will run 8 tests
-echo "1..8"
+# TAP test plan: we will run 9 tests
+echo "1..9"
 
 c_test test-ids
 c_test test-pending-call-dispatch
@@ -78,3 +78,4 @@ c_test test-privserver-client
 c_test test-shutdown
 py_test test-activation-forking.py
 c_test test-autolaunch
+c_test test-multithread-client


I get 

PASS: run-test.sh 1 test-ids
PASS: run-test.sh 2 test-pending-call-dispatch
PASS: run-test.sh 3 test-pending-call-timeout
PASS: run-test.sh 4 test-threads-init
PASS: run-test.sh 5 test-privserver-client
PASS: run-test.sh 6 test-shutdown
PASS: run-test.sh 7 test-activation-forking.py
PASS: run-test.sh 8 test-autolaunch
PASS: run-test.sh 9 test-multithread-client
PASS: run-test-systemserver.sh 1 test-expected-echo-fail --print-reply --dest=org.freedesktop.DBus.TestSuiteEchoService /org/freedesktop/TestSuite org.freedesktop.TestSuite.Echo string:hi (Saw "DBus.Error" in output as expected)
PASS: run-test-systemserver.sh 2 test-wait-for-echo.py
============================================================================
Testsuite summary for dbus 1.11.1
============================================================================
# TOTAL: 11
# PASS:  11
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0

Supporting tests in test/name-test on windows/wine depends on the patches of bug 92899

(In reply to Ralf Habacker from comment #14)
> Supporting tests in test/name-test on windows/wine depends on the patches of
> bug 92899

Will it be merged in test/name-test directory currently for Linux support.
Thanks

I think adding it for Linux support will be good since we have recently fixed run-test.sh script for name-test tests in master branch.

Comment on attachment 121438 [details] [review]
[Patch] Updated patch with compilte time warning removed with latest master branch

Review of attachment 121438 [details] [review]:
-----------------------------------------------------------------

Found some remaining name issues, please fix.

::: test/Makefile.am
@@ +324,4 @@
>  	data/valid-service-files/org.freedesktop.DBus.TestSuiteSegfaultService.service.in \
>  	data/valid-service-files/org.freedesktop.DBus.TestSuiteShellEchoServiceFail.service.in \
>  	data/valid-service-files/org.freedesktop.DBus.TestSuiteShellEchoServiceSuccess.service.in \
> +	data/valid-service-files/org.freedesktop.DBus.TestSuite.Multithreaddbus.service.in \

Please use org.freedesktop.DBus.TestSuite.MultiThreadServer.service.in

::: test/data/valid-service-files/org.freedesktop.DBus.TestSuite.Multithreaddbus.service.in
@@ +1,1 @@
> +[D-BUS Service]

filename should be org.freedesktop.DBus.TestSuite.MultiThreadServer.service.in

@@ +1,2 @@
> +[D-BUS Service]
> +Name=org.freedesktop.DBus.TestSuite.Multithreaddbus

Please use  

Name=org.freedesktop.DBus.TestSuite.MultiThreadServer

::: test/name-test/test-multithread-client.c
@@ +1,5 @@
> +#include <config.h>
> +#include<pthread.h>
> +#include "../test-utils.h"
> +
> +#define DBUS_MULTITHREAD_SERVICE "org.freedesktop.DBus.TestSuite.Multithreaddbus"

The most similar service is Name=org.freedesktop.DBus.TestSuite.PrivServer which uses camel case names 

org.freedesktop.DBus.TestSuite.PrivServer

soe it should be 

org.freedesktop.DBus.TestSuite.MultiThreadServer

@@ +2,5 @@
> +#include<pthread.h>
> +#include "../test-utils.h"
> +
> +#define DBUS_MULTITHREAD_SERVICE "org.freedesktop.DBus.TestSuite.Multithreaddbus"
> +#define DBUS_MULTITHREAD_PATH "/org/freedesktop/DBus/TestSuite/Multithreaddbus"

see above

@@ +18,5 @@
> +  exit (1);
> +}
> +
> +static void*
> +send_multithread_data()

wrong solution. 

From http://man7.org/linux/man-pages/man3/pthread_create.3.html the prototype of pthread_create():

pthread_create is defined as: 

int pthread_create(pthread_t *thread, const pthread_attr_t *attr,
                          void *(*start_routine) (void *), void *arg);

where the start routine need to be of type void *start_routine(void *)

::: test/name-test/test-multithread-server.c
@@ +18,5 @@
> +  DBusError error;
> +  char *ack="DataRecieved";
> +  dbus_error_init (&error);
> +  if (dbus_message_is_method_call (msg,
> +				"org.freedesktop.DBus.TestSuite.Multithreaddbus",

please use org.freedesktop.DBus.TestSuite.MultiThreadServer

@@ +19,5 @@
> +  char *ack="DataRecieved";
> +  dbus_error_init (&error);
> +  if (dbus_message_is_method_call (msg,
> +				"org.freedesktop.DBus.TestSuite.Multithreaddbus",
> +				"MultithreadData"))

please use MultiThreadData

@@ +55,5 @@
> +
> +  loop = _dbus_loop_new ();
> +  test_connection_setup (loop, dbus_conn);
> +
> +  dbus_bus_request_name(dbus_conn, "org.freedesktop.DBus.TestSuite.Multithreaddbus", 0, &error);

see above

@@ +66,5 @@
> +  dbus_vtable->unregister_function = NULL;
> +  dbus_vtable->message_function = handler_function;
> +
> +  if(!dbus_connection_register_object_path(dbus_conn,
> +					"/org/freedesktop/DBus/TestSuite/Multithreaddbus",

see above

(In reply to amit tewari from comment #16)
> I think adding it for Linux support will be good since we have recently
> fixed run-test.sh script for name-test tests in master branch.

Would be nice, unfortunally the patch in the current state is incomplete because it is missing the make check integration (for autotools: patch fragment from comment 8) and cmake (need some patching)

(In reply to Ralf Habacker from comment #18)
> (In reply to amit tewari from comment #16)
> > I think adding it for Linux support will be good since we have recently
> > fixed run-test.sh script for name-test tests in master branch.
> 
> Would be nice, unfortunally the patch in the current state is incomplete
> because it is missing the make check integration (for autotools: patch
> fragment from comment 8) and cmake (need some patching)

see bug 93976 for cmake support running tests in name-test dir.

Created attachment 121484 [details] [review]
Updated patch with handled naming convention and pthread prototype

(In reply to Ralf Habacker from comment #19)
> (In reply to Ralf Habacker from comment #18)
> (In reply to amit tewari from
> comment #16)
> > I think adding it for Linux support will be good since we
> have recently
> > fixed run-test.sh script for name-test tests in master
> branch.
> 
> Would be nice, unfortunally the patch in the current state is
> incomplete
> because it is missing the make check integration (for
> autotools: patch
> fragment from comment 8) and cmake (need some patching)
> see bug 93976 for cmake support running tests in name-test dir.

I understand now it is important for this bug 93976 to get fixed for this testcase to merge.
but I can run  test cases separately from name-test dir using "make check" in name-test dir, if I run configure using "--enable-embedded-tests" option.

I think since the bug 93976 changes will apply to all testcases within name-test dir, we can merge this testcase to name-test dir so that once the blocking bug is closed multithread testcase will also get included and run along with other name-test testcases, this way multithread testcase will be preserved for other developers also.
Thanks

Comment on attachment 121484 [details] [review]
Updated patch with handled naming convention and pthread prototype

Review of attachment 121484 [details] [review]:
-----------------------------------------------------------------

::: test/name-test/test-multithread-client.c
@@ +18,5 @@
> +  exit (1);
> +}
> +
> +static int
> +send_multithread_data()

still an issue http://man7.org/linux/man-pages/man3/pthread_create.3.html stated to use the following prototype style

   static void *thread_start (void *arg)

whichs means to change the function name to 

static void *
send_multithread_data (void *args)

Please note that dbus uses <function-name><space>( style; there are additional locations in this file affected.

@@ +71,5 @@
> +
> +}
> +
> +int
> +main(int argc, char** argv)

space after function name missing

@@ +78,5 @@
> +  pthread_t mythread[MAXTHREAD];
> +  dbus_threads_init_default();
> +  for(i = 0; i < MAXTHREAD; i++)
> +  {
> +    if(pthread_create(&mythread[i],NULL,(void *)send_multithread_data,NULL)!=0)

no cast, use type safe &send_multithread_data

::: test/name-test/test-multithread-server.c
@@ +11,5 @@
> +  exit (1);
> +}
> +
> +static DBusHandlerResult
> +handler_function(DBusConnection *conn, DBusMessage *msg, void *user_data)

space after function name missing, please fix also the additional related locations in this file.

Created attachment 121487 [details] [review]
Updated patch with handled coding guideline and pthread prototype

Comment on attachment 121487 [details] [review]
Updated patch with handled coding guideline and pthread prototype

Review of attachment 121487 [details] [review]:
-----------------------------------------------------------------

For the commit message: it's spelled "D-Bus". "dbus" is the name of the reference implementation of D-Bus; "Dbus" doesn't exist.

General high-level questions and points about these:

Why is this particular test interesting? You're sending some arbitrary number of messages, in a blocking (synchronous) way, from multiple sender threads, to a recipient out-of-process. Did this exhibit a bug for you? Are you using some other code for which this is a simplified emulation? Is there some particular piece of code that gets coverage this way but not in any of the existing tests? What makes this better than any of the million other possible tests?

I would prefer new tests to start their own dbus-daemon, like test/dbus-daemon.c does, instead of running in name-test/ and relying on a separate dbus-daemon activating an out-of-process service.

I would prefer new tests to do their messaging by "talking to themselves" - running the service side in-process, like test/dbus-daemon.c does, instead of in a separate activated process like this one does - because that makes it *much* easier to run both client and service sides under a debugger like gdb, or a QA tool like valgrind or helgrind.

I would prefer new tests to use GTest, like test/dbus-daemon does; or failing that, output in TAP format <http://testanything.org/>.

If you are adding new tests to name-test/ despite those considerations, please edit run-test.sh to enable them. There is nothing magic about the line that says "1..8": it's a TAP <http://testanything.org/> "test plan". Patching it to say "1..9" would be fine. I don't want to add new tests that have not been run in the test environment by their authors, and in general I don't want to add new tests that will not be run automatically by the test framework, unless there's a very good reason why that can't be done (for instance needing manual setup, like test/manual-authz.c, or manual interpretation of the results, like test/manual-paths.c).

::: test/name-test/test-multithread-client.c
@@ +1,1 @@
> +#include <config.h>

New code needs a copyright statement and a license: either the same dual-license as dbus/dbus-connection.c (GPL-2+|AFL-2.1), or the same permissive license as test/dbus-daemon.c (the same MIT/X11 license as Expat). Either is fine; for new modules like this one I'd prefer the permissive license, so that it can be copied into other projects and/or used as an example.

@@ +1,2 @@
> +#include <config.h>
> +#include<pthread.h>

This is weird syntax, please do the conventional spacing.

@@ +3,5 @@
> +#include "../test-utils.h"
> +
> +#define DBUS_MULTITHREAD_SERVICE "org.freedesktop.DBus.TestSuite.MultiThreadServer"
> +#define DBUS_MULTITHREAD_PATH "/org/freedesktop/DBus/TestSuite/MultiThreadServer"
> +#define DBUS_MULTITHREAD_IFACE DBUS_MULTITHREAD_SERVICE

Please don't use the DBUS/dbus/DBus namespace for things that are not API. These constants could be called MULTITHREAD_WHATEVER or TEST_MULTITHREAD_WHATEVER.

@@ +5,5 @@
> +#define DBUS_MULTITHREAD_SERVICE "org.freedesktop.DBus.TestSuite.MultiThreadServer"
> +#define DBUS_MULTITHREAD_PATH "/org/freedesktop/DBus/TestSuite/MultiThreadServer"
> +#define DBUS_MULTITHREAD_IFACE DBUS_MULTITHREAD_SERVICE
> +#define MAXTHREAD 5
> +pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;

If you're going to hard-code use of pthreads (as opposed to using some threading abstraction like the one from GLib), then this file needs to be conditionally added to lists of executables in both CMake and Autotools build systems, so that it's only compiled on Unix systems (or equivalently, isn't compiled for Windows, where pthread is not a standard platform API, and we use native Win32 thread support).

@@ +15,5 @@
> +  va_start (args, message);
> +  vfprintf (stderr, message, args);
> +  va_end (args);
> +  exit (1);
> +}

I'd prefer this to add a "\n" automatically, so that calls to die() don't have to remember to include one (which you have not done consistently).

@@ +25,5 @@
> +  DBusConnection *dbus_conn;
> +  DBusMessage *reply;
> +  DBusError error;
> +
> +  const long long DataSize =  1024*1024;

Please prefer to use portable types: size_t for memory sizes, something like dbus_uint64_t for general large integers.

Please use D-Bus' usual naming conventions: variables_are_lower_case_with_underscores, TypesAreCapitalized, CONSTANTS_ARE_UPPER_CASE_WITH_UNDERSCORES, and nothing uses Both_Caps_And_Underscores. For instance this should be data_size.

@@ +33,5 @@
> +  char *ack;
> +  dbus_error_init (&error);
> +  dbus_conn =  dbus_bus_get (DBUS_BUS_SESSION, &error);
> +  if (dbus_error_is_set (&error))
> +    die ("Client:couldn't access session bus\n");

This should print the name and human-readable message from 'error'.

@@ +36,5 @@
> +  if (dbus_error_is_set (&error))
> +    die ("Client:couldn't access session bus\n");
> +
> +  for (i = 0 ; i < Num_Msg ; i++ )
> +  {

Please indent all new code in GNU style: {} are indented 2 spaces further than the "for", "if", etc., and the contents of {} are indented 2 more spaces. For instance see test/dbus-daemon.c, which is correctly-indented.

@@ +37,5 @@
> +    die ("Client:couldn't access session bus\n");
> +
> +  for (i = 0 ; i < Num_Msg ; i++ )
> +  {
> +    char *data = (char*)malloc (DataSize);

This cast is unnecessary: ISO C allows void * to be assigned to char * without a cast.

Please prefer to use dbus_malloc() and similar when writing for libdbus.

@@ +40,5 @@
> +  {
> +    char *data = (char*)malloc (DataSize);
> +    if (!data)
> +      die ("couldn't malloc data\n");
> +    memset (data, 'a', DataSize*sizeof(char));

data_size * sizeof (char)

@@ +51,5 @@
> +    dbus_message_append_args (message, DBUS_TYPE_INT32, &i, DBUS_TYPE_STRING, &data, DBUS_TYPE_INVALID);
> +    reply = dbus_connection_send_with_reply_and_block (dbus_conn, message, -1, &error);
> +    if (dbus_error_is_set (&error))
> +    {  die ("Client:couldn't send message: %s\n", error.message);
> +      dbus_message_unref (message);

This die() call is particularly weirdly indented. It should print the error name and message. I'm pretty sure test-utils has a convenience function to assert that a DBusError is unset...

@@ +55,5 @@
> +      dbus_message_unref (message);
> +    }
> +
> +    if (!dbus_message_get_args (reply, &error, DBUS_TYPE_INT32, &Ret_Id, DBUS_TYPE_STRING, &ack, DBUS_TYPE_INVALID))
> +      die ("Client: Error getting ARG from Reply");

This should print the error name and message.

@@ +58,5 @@
> +    if (!dbus_message_get_args (reply, &error, DBUS_TYPE_INT32, &Ret_Id, DBUS_TYPE_STRING, &ack, DBUS_TYPE_INVALID))
> +      die ("Client: Error getting ARG from Reply");
> +
> +    if (!(i == Ret_Id && ack && !strcmp (ack, "DataRecieved")))
> +      die ("Invalid reply from server");

If you're going to include magic strings in your protocol, please spell "received" correctly :-)

@@ +59,5 @@
> +      die ("Client: Error getting ARG from Reply");
> +
> +    if (!(i == Ret_Id && ack && !strcmp (ack, "DataRecieved")))
> +      die ("Invalid reply from server");
> +    printf ("Client : Reply received  thread = %lu\n", (unsigned long)pthread_self ());

Tests should not output junk on stdout: I'm trying to move our tests towards TAP format, which parses stdout. If you need to output progress indications, use stderr.

Printing to stdout or stderr from non-main threads seems very questionable: at best you'll get randomly interleaved lines, and at worst you'll make all your threads' operations effectively serialized (because they're waiting for the lock that libc presumably has around each FILE * to avoid crashes), removing the value of testing multi-threaded operation.

@@ +61,5 @@
> +    if (!(i == Ret_Id && ack && !strcmp (ack, "DataRecieved")))
> +      die ("Invalid reply from server");
> +    printf ("Client : Reply received  thread = %lu\n", (unsigned long)pthread_self ());
> +
> +    if (reply)

if (reply != NULL)

@@ +74,5 @@
> +int
> +main (int argc, char** argv)
> +{
> +  int i = 0;
> +  pthread_t mythread[MAXTHREAD];

I'd prefer a blank line between declarations and code, to make it more obvious that we're working in what is basically C89 (no interleaving declarations with code).

@@ +75,5 @@
> +main (int argc, char** argv)
> +{
> +  int i = 0;
> +  pthread_t mythread[MAXTHREAD];
> +  dbus_threads_init_default ();

This should not have been necessary since 1.8.

@@ +79,5 @@
> +  dbus_threads_init_default ();
> +  for (i = 0; i < MAXTHREAD; i++)
> +  {
> +    if (pthread_create (&mythread[i], NULL, send_multithread_data, NULL) != 0)
> +      die ("error creating thread");

This is an example of forgetting to put the \n on the die() call.

This should presumably mention strerror() somewhere.

::: test/name-test/test-multithread-server.c
@@ +45,5 @@
> +  DBusObjectPathVTable *dbus_vtable;
> +  DBusError error;
> +  DBusLoop *loop;
> +
> +  dbus_threads_init_default ();

This hasn't been necessary since 1.8.

@@ +49,5 @@
> +  dbus_threads_init_default ();
> +  dbus_error_init (&error);
> +
> +  dbus_conn =  dbus_bus_get (DBUS_BUS_SESSION, &error);
> +  if(dbus_error_is_set (&error))

if (...

@@ +61,5 @@
> +    die ("couldn't request name: %s", error.message);
> +
> +  dbus_vtable = malloc (sizeof(DBusObjectPathVTable));
> +  if (!dbus_vtable)
> +    die ("error malloc vtable");

This doesn't need to be malloc()'d, it could be on the stack.

(In reply to amit tewari from comment #21)
> I understand now it is important for this bug 93976 to get fixed for this
> testcase to merge.

That is not required.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/dbus/dbus/issues/138.