Skip to content

PS-9697 C++ KMIP client library added #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lukin-oleksiy
Copy link
Contributor

The "kmipclient" C++ library implementation is added, that wraps low-level "kmip.h" calls and bypasses mid-level "kmip_bio" level.

@lukin-oleksiy lukin-oleksiy force-pushed the PS-9697-POC-of-kmip-cpp branch 6 times, most recently from ecb590a to 38e07d8 Compare April 16, 2025 09:55
include/Key.hpp
src/StringUtils.cpp
src/StringUtils.hpp
include/Logger.hpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not a CMake expert, but I don't think we should list header files in the list of library files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general case we should. Header files will definitely be used when the CMake project generator is, say, "Visual Studio" or "XCode". For "Linux Makefiles", this may not be the case though as there header dependencies are generated via -MD compiler options. Also, not sure about "Ninja" generator - it may use another approach to generate dependency trees, so there including headers may also be useful.


} // namespace ve

#else // C++23 or later
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I am struggling to understand what is the difference between implementations of expected for the C++ standard is less than C++23 branch and the C++23 or later branch. If such a difference exists, please describe it in the source file as comments.

Why in the first place do we need to implement expected when C++23 is supported? Could we just use an alias template to the std::expected?

kmipclient::AttributesFactory::parse (Attribute *attribute, size_t attribute_count)
{
attributes_t res;
return res;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be unimplemented?

Comment on lines +106 to +107
int state = 0;
int secret_type = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider changing types for state and secret_type from int to kmip_entity_state and kmip_secret_type respectively.

ids_t res;
for (int i = 0; i < locate_result.ids_size; ++i)
{
res.emplace_back (locate_result.ids[i]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems we have double copying. First, we copy strings pld->unique_ids to locate_result.ids, then - from locate_result.ids to res. Can we eliminated locate_result.ids and copy straight from pld->unique_ids to res?

else
last_result.result_message[0] = 0;

FILE *mem_stream = open_memstream (&bp, &size);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please consider using std::ostringstream instead of FILE* for formatted output.

The "kmipclient" C++ library implementation is added, that wraps low-level
"kmip.h" calls and bypasses mid-level "kmip_bio" level.
@lukin-oleksiy lukin-oleksiy force-pushed the PS-9697-POC-of-kmip-cpp branch from 38e07d8 to eb16cf2 Compare June 26, 2025 09:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants