david-maw / StreamSSL

The StreamSSL sample described in CodeProject
Other
47 stars 24 forks source link

Question: Include folder in StreamClient / StreamServer #63

Closed 0ric1 closed 5 years ago

0ric1 commented 5 years ago

Hello, is it possible to create include folders (as in Common) in StreamClient / StreamServer for the .h files that should be visible to the user: The StreamClient/Include folder would contain: ActiveSock.h, EventWrapper.h and SSLClient.h. The StreamServer/Include would contain SSLServer.h, PassiveSock.h, Listener.h. Thanks, Andreas.

david-maw commented 5 years ago

In principle, yes, although the other files would still be needed at build time and it would make the search path slightly longer - what would be the objective of such a change? I don't particularly object to it, just want to understand how it makes things better for the user.

0ric1 commented 5 years ago

As consumer of e.g. the StreamClient I only need the three files ActiveSock.h, EventWrapper.h, SSLClient.h (the public API) and the Common files, the other files pch.h, ... or include files that are only included by some .cpp files the consumer don't need to see - think StreamClientSSL is a library. I for example use only your StreamClientSSL in my project, as I do with other third party libraries, I add the include path to the "public" include folder of the third party library to my project - these files do I need to use your "library" the others are "implementation detail". Also it would have the same structure as in Common.

0ric1 commented 5 years ago

I created a branch for static library and include folder and pull requests see https://github.com/david-maw/StreamSSL/pull/66 and https://github.com/david-maw/StreamSSL/pull/67

david-maw commented 5 years ago

Yep, it all looks good to me, I merged it - I'm thinking of renaming the Lib projects something lime SSLClient and SSLServer (or maybe SSLClientLib and SSLServerLib) to make it clearer what comes from where. Do you have an opinion on whether that would help make things clearer?

0ric1 commented 5 years ago

No, I am not a fan of Lib in the name because the generated .lib would get the name SSLServerLib.lib - this is what a newby would do with her/his first library. Tomorrow I make a shared library and then the name would be SSLServerLib.dll. If you look at other third party projects there is almost never a lib in the name e.g. https://github.com/google/benchmark a bad example is e.g. https://github.com/ppescher/resizablelib that results in ResizableLib.lib. It would be better to call the applications ...Test (as done in https://github.com/google/benchmark) or ...Demo or ...Sample (as done in google test), I would prefer Sample, than we should rename the Demo folder to samples too.

david-maw commented 5 years ago

ok, II didn't much like ...lib.lib either, but SSLClient and SSLServer seem more meaningful than the current StreamClient and StreamServer. Changing from Demo to Samples strikes me as reasonable; I've also used "examples" for similar code in past projects.

david-maw commented 5 years ago

So do SSLClient and SSLServer work for you? If so I'll go ahead and change Demo to Samples as well.

0ric1 commented 5 years ago

So do SSLClient and SSLServer work for you? If so I'll go ahead and change Demo to Samples as well. I agree, SSLClient/Server is more meaningful, also the files are named SSLClient/Server.h/cpp so the projects should have the same names.

david-maw commented 5 years ago

ok, done.

0ric1 commented 5 years ago

ok, done. You renamed only the project name, but the folder and the .vcxproj and .filters have still the old name, that is confusing.

david-maw commented 5 years ago

Yes it was, sorry about that, I fixed it. It took a lot of messing about in project files so if you spot anything amiss, let me know.

0ric1 commented 5 years ago

We use in All configuration/General/C++ Language Standard: Preview - Features from the Latest C++ Working Draft (std:c++latest) Conformance mode: Yes(/permissive-) Enforce type conversion rules: Yes (/Zc:rvalueCast)

Conformance mode on SSLCLient and SSLServer does not compile.

x64/Release/ C/C++/Optimization: Inline Function Expansion: Any Suitable /Ob2 Favor Size Or Speed: Favor fast code /Ot Omit Frame Pointers: Yes /Oy C/C++/Code Generation: Enable String Pooling: Yes /GF

0ric1 commented 5 years ago

The ToolsVersion="15.0" is removed in VS2019 .vcxproj files.

david-maw commented 5 years ago

I'm not sure what you mean by "The ToolsVersion="15.0" is removed in VS2019 .vcxproj files", it still seems to be there. Did you mean it should be removed for some reason? VS2019 seems to parse those files correctly - am I missing something?

0ric1 commented 5 years ago

Create a new e.g vonsole application with VS2019, than look into the .vcxproj with a text editor, you will see the line: <Project DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> no more ToolsVersion="15.0" as with VS2017. Also there are new entries: <VCProjectVersion>16.0</VCProjectVersion> and <ImportGroup Label="Shared"> </ImportGroup>

david-maw commented 5 years ago

I've checked in a set of updates so it compiles with Draft (std:c++latest) Conformance mode: Yes(/permissive-) Enforce type conversion rules: Yes (/Zc:rvalueCast)

Mostly just casts so the same code is emitted as before. That leaves a small risk the compiler implementation will change and break the code, but I doubt it will because doing that would impact too much existing code.

david-maw commented 5 years ago

I see what you were getting at with the Toolsversion comment, but it seems to me VS owns that file and if the VSS authors wanted to make changes on migration they perfectly well could and unless there's some problem I'm not inclined to second guess them. Is there a problem?

0ric1 commented 5 years ago

No problem, was only for consistency - it is only a left over from the old Studio.

david-maw commented 5 years ago

ok thanks, I'll change it if I'm in the area for something else, you should feel free to do the same if you happen to be editing the vcxproj file. As far as I can tell it has no effect on VS2017 or even 2015 either.