Hi Bob,
thanks for your contribution. In addition to the comments by Daniel and
Peter, my comments are in line:
On 19.03.2014 22:37, Bob Kast wrote:
> Libssh2.h:
>
> In Windows, a socket is of type SOCKET, not int.
>
Please post this as a seperate patch as it is not WinCNG-specific.
> Libssh2_priv.h:
>
> Redundant #define inline
>
It might be there for a reason. Please do not forget that there are
other Windows-compilers except Visual Studio. For example, I use MinGW
and MinGW-w64 to compile libssh2. It might be worth to add an #ifndef
around the #define in that case.
> Openssl.c:
>
> You shouldn’t need Openssl to compile if you aren’t selecting it.
>
That makes perfect sense with Peter's recent changes to the backend
selection. We just need to make sure that LIBSSH2_OPENSSL is defined as
the default within all buildfiles in order to be backwards compatible.
> Wincng.c:
>
> Putting the #pragma for the libraries works for both DLL and LIB versions.
>
This is rather compiler-specific to Visual Studio than
platform-specific. The #ifdef WIN32 ist not sufficient in that case,
since MinGW gcc compilers won't understand the pragma statements. I
would rather suggest adding the libraries to the AdditionalDependencies
element of the Link section within your Visual Studio project files.
> _libssh2_wincng_hash_update: the parameter needs to be const to match.
> It is interesting that C just gives a warning for that.
>
But is casting from (const unsigned char *) to (unsigned char *)
actually a real fix for this or just hiding things?
> pPaddingInfo value is undefined. Doc says it must be NULL if not used.
>
Good catch, that's correct.
> Wincng.h:
>
> STATUS_SUCCESS unfortunately not defined if using non-driver includes.
>
I will have to take a look at this. It works for MinGW. Please give me
some time over the weekend to setup my own Visual Studio 2012 to build
libssh2 and verifiy your suggestions.
> _libssh2_wincng_hash_ctx and _libssh2_wincng_key_ctx: I found this
> confusing. Kind of a circular #define. I think this is more of what
> was intended.
>
This change is okay with me. I think I used defines, because it the
approach was used within other places of libssh2. Daniel, Peter, what do
you think?
> Forward declarations: without these the compiler complains for each call.
>
MinGW gcc does not complain and the other backends do not have them.
Again, please allow me some time to verify this.
> Libssh2_config.h:
>
> Pragma to suppress “possible loss of data” warnings.
>
A compiler-specific flag would could also be put into the Visual Studio
project configuration.
> New Files:
>
> -Libssh2.sln – solution file
>
> -Libssh2.vcxproj, libssh2.vcxproj.filters – project files for libssh2
>
> -Tests.vcxproj, tests.vcxproj.filters – project files for tests – I
> haven’t tested this.
>
> These are specifically for VS2013 and make it easy to create DLL/LIB,
> Debug/Release, 32/64 bit builds.
>
I think we should rather wait for Alexander Lamaison's CMake files since
they will allow us to create generic VS project files that are not tied
to a specific VS and SDK version. For example, I still use VS 2012 for
my own projects.
Thanks again.
Best regards,
Marc
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2014-03-20