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
