[Speex-dev] Strange style of includes

[Jean-Marc Valin]

Hi

On 10-06-21 07:24 PM, Pavel Pavlov wrote:
> I'm just curious, who and why came up with that strange way to write includes:

I did. Because I think that's the way to go.

> #ifdef _BUILD_SPEEX
> # include "speex_types.h"
> #else
> # include<speex/speex_types.h>
> #endif
>
> I personally consider it a bug. If I use speex then obviously I should not have _BUILD_SPEEX defined.

So far so good.

> The broken behavior happens in this simple scenario:
> 1) I check out latest git version to ./src/speex
> 2) My system has an older "release" version installed to regular install location.

So problem so far.

> 3) I want to use the version I just checked out, I include filed this way: "...whatever/src/speex/speex_bits.h"

Wrong! What you need to do is add "../whatever/src" to your include path 
(e.g. -I with gcc)

> Broken behavior: instead of using files from check out version, speex includes will include system installed versions of old speex! That's just bad, I've never seen anything like that

Fix your code -- I've never seen anything like that ;-)

> I'll attempt to answer the "why" question... speex also has Symbian port, and just like everything is ugly crap in Symbian world, their version of gcc is also broken and you can't include headers relative to the directory of current file and that approach with<path/to/file.h>  would probably fix that problem in Symbian. I think _build_speex thing should be removed, plus have_config_h usually has the same meaning: to indicate that the library is being build and not used.

I think you managed to get it wrong all the way. FYI, I had nothing to 
do with the Symbian port and had no idea that Symbian had include issues.

> Speex code uses EXPORT to mark public api functions. If default visibility is hidden then it's all ok,

Exactly. That's the idea.

> but I'd personally use static for private functions if they aren't referenced across different files.

Yes, I know what "static" is for. If I forgot functions that should be 
static, you can send a patch.

> I also use one file compilation: libspeex.c that includes all of speex so that speex can be used in any library by simply adding reference to that .c file to a makefile (just like pthreads does). The is a minor problem though:  nb_celp.c and sb_celp.c redefine the same macro with different values: LSP_MARGIN, LSP_DELTA1, LSP_DELTA2 (which result in a warning) and _kiss_fft_guts.h doesn't have header guards (results in errors).

kiss_fft could probably use some include guards. Other than that, if you 
want to do things like include all .c in the same file, just #undef the 
what you need.

Cheers,

	Jean-Marc