Ticket #954 (new defect)

Opened 6 years ago

Last modified 5 years ago

[TRIVIAL] Cannot include opensync headers without first including opensync.h

Reported by: ianmartin Owned by:
Priority: normal Milestone: OpenSync 0.40
Component: OpenSync: Application API Version: 0.38
Severity: normal Keywords:
Cc:

Description

Lots of headers use OSYNC_EXPORT without #including opensync.h

Any file that includes one of these headers without first including opensync.h breaks.

Files using OSYNC_EXPORT:

find . -name '*.h'  |xargs grep -l  OSYNC_EXPORT

of those files which ones include opensync.h

 find . -name '*.h'  |xargs grep -l  OSYNC_EXPORT |xargs grep -l opensync.h

.opensync/opensync_list.h

Attachments

Ticket954.tar.gz (709 bytes) - added by ChrisH 5 years ago.

Change History

comment:1 Changed 5 years ago by bricks

  • Summary changed from Cannot include opensync headers without first including opensync.h to [TRIVIAL] Cannot include opensync headers without first including opensync.h

comment:2 Changed 5 years ago by ChrisH

Just to be sure, is that a valid patch for this ticket?

If yes I could continue with the script to check and correct this.

svn diff
Index: opensync/helper/opensync_hashtable.h
===================================================================
--- opensync/helper/opensync_hashtable.h        (revision 5823)
+++ opensync/helper/opensync_hashtable.h        (working copy)
@@ -22,6 +22,8 @@
 #ifndef OPENSYNC_HASHTABLE_H_
 #define OPENSYNC_HASHTABLE_H_

+#include "opensync/opensync.h"
+
 typedef void (*OSyncHashtableForEach) (const char *uid, const char *hash, void *user_data);

 /**

comment:3 follow-up: ↓ 4 Changed 5 years ago by dgollub

Maybe we should declare that direct includes of headers like opensync/$component/opensync_$impl.h is not allowed. And just allow inlcudes of <opensync/opensync-$component> (the high-level component headers .. and only add "#include "opensync/opensync.h" to the high-level headers ...

What do you think?

We could also define a macro for those opensync/$component/opensync_$impl.h which warns about direct inclusion (i guess this is common practive in various APIs - e.g. Qt, glib or so)

comment:4 in reply to: ↑ 3 Changed 5 years ago by bricks

Replying to dgollub:

Maybe we should declare that direct includes of headers like opensync/$component/opensync_$impl.h is not allowed. And just allow inlcudes of <opensync/opensync-$component> (the high-level component headers .. and only add "#include "opensync/opensync.h" to the high-level headers ...

That would be really good. I ran into the same problem with C++. If you include only the "low level" headers extern C isn't defined and the linker can't find the right symbols. Therefore it would be the best solution to not allow direct inclusion of low level headers.

What do you think?

We could also define a macro for those opensync/$component/opensync_$impl.h which warns about direct inclusion (i guess this is common practive in various APIs - e.g. Qt, glib or so)

Didn't know that such a macro exists :)

Changed 5 years ago by ChrisH

comment:5 Changed 5 years ago by ChrisH

Something like this?

make
gcc -o helloWorld helloWorld.c
In file included from helloWorld.c:2:
helloWorld1.h:2:2: warning: #warning Do not include helloWorld1.h direct, include helloWorld.h

I will attach a tarball with some simple files as reproducer,

comment:6 Changed 5 years ago by dgollub

  • Status changed from new to assigned
  • Component changed from OpenSync to OpenSync: Application API

Will have a look ...

comment:7 Changed 5 years ago by dgollub

  • Owner dgollub deleted
  • Status changed from assigned to new

Looks good to me.

Could some one provide a patch for all opensync headers which get installed in the system environment.

Note: See TracTickets for help on using tickets.