pupnp / pupnp

libupnp: Build UPnP-compliant control points, devices, and bridges on several operating systems.
https://pupnp.github.io/pupnp
BSD 3-Clause "New" or "Revised" License
352 stars 117 forks source link

Some installed headers cannot be included #56

Closed jcowgill closed 2 years ago

jcowgill commented 6 years ago

Some installed include files error out when included. They should be fixed, or maybe some of the headers are private?

TemplateInclude.h

TemplateInclude.h:147:1: error: parameter 1 has incomplete type
 EXPAND_CLASS_MEMBERS(CLASS)
 ^~~~~~~~~~~~~~~~~~~~
TemplateInclude.h:147:1: warning: return type defaults to ‘int’ [-Wimplicit-int]
TemplateInclude.h: In function ‘EXPAND_CLASS_MEMBERS’:
TemplateInclude.h:147:1: error: parameter name omitted
TemplateInclude.h:150:0: error: expected ‘{’ at end of input

TemplateSource.h

TemplateSource.h:209:10: fatal error: config.h: No such file or directory
 #include "config.h"
          ^~~~~~~~~~

list.h

list.h:39:20: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘void’
 static UPNP_INLINE void INIT_LIST_HEAD(struct list_head *list)
                    ^~~~

upnpdebug.h

upnpdebug.h:40:10: fatal error: ThreadPool.h: No such file or directory
 #include "ThreadPool.h"
          ^~~~~~~~~~~~~~
jcowgill commented 6 years ago

OK, I see that the Template* files are special and not supposed to be included directly. I think the other 2 should still be fixed though.

mrjimenez commented 6 years ago

Hi James,

Can you please send me the configure command you are using? I tried compiling with and without debugging information, but I got no errors, just a few warnings.

Best regards, Marcelo.

ffontaine commented 6 years ago

Dear Marcelo,

The issue is not linked to configure but to the "public" headers we install in /usr/include/upnp. Indeed, we should remove the install of TemplateInclude.h and TemplateSource.h, Concerning list.h, we should add an include to UpnpGlobal.h in this file so UPNP_INLINE will be declared. Finally, we should remove the ThreadPool.h inclusion in upnpdebug.h as ThreadPool.h is now private.

I will try to fix these issues.

Best Regards,

Fabrice

ffontaine commented 6 years ago

In fact, we can't remove Templatexxx files but perhaps we should find a way to let the user knows that these headers should not be included directly, should we move their installation in a "upnp/private" directory? In this case, I think we should also move list.h as it is only used by TemplateInclude.h.

mrjimenez commented 6 years ago

Hi Folks,

Fabrice is right, if I recall correctly, at least TemplateInclude.h must stay. Also, he is right about list.h, even if no UPnP file includes it directly, we must observe that TemplateInclude.h is a class generator, so any class that uses a list type will automatically use list.h.

I tend to think that the "upnp/private" folder solution is totally acceptable for the next release. Leaves it pretty clear that these files are not for public inclusion.

Regards, Marcelo.

ffontaine commented 6 years ago

OK Marcelo, I will send a PR tomorrow so we can review the "upnp/private" folder solution

ffontaine commented 6 years ago

Finally, TemplateInclude can't easily be moved in upnp/private as it includes ixml.h which should remain in the main "upnp" public directory so I would suggest to keep Template files in their current location. Perhaps, we should just add a comment to those files to warn the user that they should not be included directly?

ukleinek commented 6 years ago

you can add some cpp magic that issues a warning or an error if the file is included directly.

Vollstrecker commented 3 years ago

I think this one can get closed. At least the first 2 files don't even exist anymore.

mrjimenez commented 3 years ago

I don't know, maybe @jcowgill can test and answer. What I know is:

  1. Templates are gone.
  2. The list API has been modified, so list.h is a totally different file today.
  3. upnpdebug.h no longer includes ThreadPool.h.
Vollstrecker commented 2 years ago

So three reasons and almost a year of no response - closing.