[patch] strigi (trunk) fixes for msvc

[Jos van den Oever]

Hi Jaroslow,

2008/1/30, Jarosław Staniek <js at iidea.pl>:
> For review:
Can I have some context on this patch? It's rather long and ugly. I
guess you're trying to compile strigi on windows using a microsoft
compiler. What version of both are you using? I'm being a bit harsh
but my problem is that you are reusing a block of #ifdefs over and
over and sprinkling (int32_t) casts around like they are for free.

> - the main issue was that int32_t is 'signed long' and not 'int' (compare
> include/msvc/stdint.h with clucene typesdefs), so we needed to add some ifdefs
int32_t is always right, so if there is an ambiguity, int32_t should be used.
Can you give a code snippet to explain what exactly is the problem?

> --- in for one case (cluceneindexreader.cpp) added a (ifdef'd) long long
> overload for Strigi::Variant's operator= because if we include variant.h
> header within a scope of different int32_t definition, we'll get linking error
A different definition of int32_t? How is that possible?

> - added casts for ambiguous overloads of AnalysisResult::addValue(), usually
> using (int32_t)
Ideally, we should have added an addValue() for each of the int types,
so unfortunately this is required at the moment. So when no int32_t
can be used in the analyzer, it is ok to cast. For cases where you
write e.g. "(int32_t)64", can you explain why that is needed?

> - there were also problems with windows.h/winsock2.h includes - to fix
> conflicting macros it we needed to include winsock2.h early enough
> (of course we cannot alter the system headers and _wanted_ to avoid altering
> clucene headers)
For now, maybe we have to live with this, but it would be nice if
you'd test and send the equivalent #ifdefs also to the clucene team.

> - added defines like HAVE_SOCKET_H for msvc as these are always available
> thanks to kdewin32 layer
What's the kdewin32 layer? Is it something you install before
compiling kdesupport?

> I have assumed that msvc target is the one dependent on kdewin32.
Eh? you lost me here.

> A number of above fixes will be reused for mingw target as well.

In short, thanks for the contribution, but can't you make it bit
easier on the eyes or, if you need to something ugly, at least add a
clear comment. Ugly code tends to be removed by people that are
blissfully unaware of porting issues and feel like cleaning up the
code.

Cheers,
Jos