Subject: RE: Patches for Windows, Wincng, Visual Studio

RE: Patches for Windows, Wincng, Visual Studio

From: Bob Kast <bob_2824_at_hotmail.com>
Date: Mon, 19 May 2014 09:18:49 -0400

Marc,

> I am holding back the following patches until we figured out an approach
to
> generate Visual Studio project files:
>
> 0001-Add-Visual-Studio-2013-solution-project-files.patch
> 0001-for-MS-VS-builds-specify-the-libraries-that-are-requ.patch

The patch: "0001-for-MS-VS-builds-specify-the-libraries-that-are-requ.patch
" has nothing to do with whether VS project files are used or not. The point
of this patch is to build into the library the instructions of what
libraries need to be linked in. Putting this in the C source has several
benefits:
- if the user is building a static library, these instructions pass through
to the project using the library. The idea is this: If a user is building an
application XYZ and links in a static library of libssh2.lib, he will also
need to specify to link in BCRYPT.lib and CRYPT32.lib. These are libraries
that he is not using directly but are being used by libssh2 so it may be
confusing. By including these statements, he needs to link only libssh2 and
libssh2 will direct what it needs under the covers.
- Windows project files typically have many configurations (release, debug,
x64, x86, etc.). The libraries to include typically are needed for all
configurations, and it is easy to have a configuration miss some parameter.
This puts it in one place instead of 4 or more places.

> 0001-Windows-library-don-t-export-externals.patch
> I checked the generated libssh2-1.dll and there were no exports besides
the
> libssh2 API functions.
> Why do you think this patch is necessary and to which build scenario
does it
> apply?

In looking over this I realize that I didn't explain the bug sufficiently.
The problem is this: If you are building a DLL, then you need to explicitly
export each entry point. When building a static library, you should not.
Libssh2 was exporting the entry points whether it was building a DLL or a
static library. To elaborate further, I was using libssh2 as a static
library, which was being linked into a project that eventually went to build
a DLL. Because the export statements were there in libssh2, they were added
as exports to my DLL. I don't feel a library should do that sort of thing in
someone else's DLL. I actually try to control my DLL exports very carefully
because my products have been hacked in the past (people disassembling and
finding security vulnerabilities). In addition to taking every precaution in
the security area (such as the secure run-time functions), I try to make
disassembling as difficult as possible by not passing through function names
as exports and using only ordinal numbers.

> Would you mind to elaborate a little bit on the changes and adapt the code
> style to the existing libssh2 code?

I apologize for not adopting the correct style. I had thought I did. Can you
give me an example of what I did wrong?

Thanks,
Bob

> -----Original Message-----
> From: libssh2-devel [mailto:libssh2-devel-bounces_at_cool.haxx.se] On Behalf
> Of Marc Hoersken
> Sent: Sunday, May 18, 2014 7:11 AM
> To: libssh2-devel_at_cool.haxx.se
> Subject: Re: Patches for Windows, Wincng, Visual Studio
>
> Hello Bob,
>
> thanks again for your patches. I applied slight modifications of the
following
> patches:
>
> 0001-formal-parameter-must-be-const-since-it-is-used-in-c.patch
> 0001-Remove-redundant-inline-define.patch
> 0001-Wincng-define-function-prototypes-for-wincng-routine.patch
> 0003-in-Windows-a-socket-is-of-type-SOCKET-not-int.patch
> 0004-a-1-bit-bit-field-should-be-unsigned-some-compilers-.patch
> 0005-openssl-should-not-compile-unless-it-is-specifically.patch
>
> On 08.04.2014 23:36, Bob Kast wrote:
> > 0001-Add-Visual-Studio-2013-solution-project-files.patch:
> >
> > I understand that you are working on a cmake system that will create
> > Visual Studio project files. Until that time, I have a patch that
> > includes project files for VS2013. It can be something temporary or it
> > can be something used as a model for creating the cmake files.
>
> I am holding back the following patches until we figured out an approach
to
> generate Visual Studio project files:
>
> 0001-Add-Visual-Studio-2013-solution-project-files.patch
> 0001-for-MS-VS-builds-specify-the-libraries-that-are-requ.patch
>
> My preference would be something like the Visual Studio files and
> generation scripts Steve Holme did for curl.
> See the following mailinglist posts to the curl-library mailinglist for
more
> information:
>
> http://curl.haxx.se/mail/lib-2014-04/0059.html
> http://thread.gmane.org/gmane.comp.web.curl.library/42126 (complete
> thread)
>
> > 0001-Use-secure-versions-of-CRT-library.patch:
> >
> > Libssh2 uses deprecated versions of the run-time library. This patch
> > updates that so they use the secure versions. For my changes to
> > correctly compile on non-Windows systems, you need to add the
> > following defines. I was not sure where these should be added:
> >
> > #define SNPRINTF snprintf
> > #define VSNPRINTF vsnprintf
>
> I think we need more feedback/information regarding the following patches
> before they can be merged:
>
> 0001-Use-secure-versions-of-CRT-library.patch
> I agree that libssh2 should use the secure string formatting functions
if they
> are available.
> I am just not sure if macros and various ifdefs are the best approach.
> Maybe we can create internal snprintf and vsnprintf wrapper functions
> instead?
> Like curlx: https://github.com/bagder/curl/blob/master/lib/curlx.h
>
> 0001-Windows-library-don-t-export-externals.patch
> I checked the generated libssh2-1.dll and there were no exports besides
the
> libssh2 API functions.
> Why do you think this patch is necessary and to which build scenario
does it
> apply?
>
> 0001-Windows-Tracing-use-OutputDebugString.patch
> It's definitely a good idea to use OutputDebugString instead of fprintf,
but
> maybe there
> should be a separate define for that instead of using the following:
> "#if defined(WIN32) && !defined(__MINGW32__)
> && !defined(__CYGWIN__)"
>
> Would you mind to elaborate a little bit on the changes and adapt the code
> style to the existing libssh2 code?
>
> Thanks in advance.
>
> Best regards,
> Marc
> _______________________________________________
> libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
_______________________________________________
libssh2-devel http://cool.haxx.se/cgi-bin/mailman/listinfo/libssh2-devel
Received on 2014-05-19