Skip to content

refactor more cpp#122

Open
gocarlos wants to merge 17 commits into
cose-wg:masterfrom
gocarlos:refactor--more-cpp
Open

refactor more cpp#122
gocarlos wants to merge 17 commits into
cose-wg:masterfrom
gocarlos:refactor--more-cpp

Conversation

@gocarlos
Copy link
Copy Markdown
Member

see commits for detailed infos

@gocarlos gocarlos requested a review from jimsch May 25, 2020 11:28
Comment thread dumper/CMakeLists.txt
endif()

target_include_directories(cose_dumper PRIVATE ../src)
target_compile_options(cose_dumper PRIVATE "-fpermissive")
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

as we are using cpp, we can overload functions -> we should get rid if macros that insert , context to the argument list IMHO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is not going to be possible to remove all of these macros as the CBOR library is still in C. I am also not too sure how much one can rely on linkers to not require functions exist, I know that one can compile each function into a separate block for MSVC but I don't know for sure that this exists in gcc and clang as they are not the compilers that I use much

@coveralls
Copy link
Copy Markdown

coveralls commented May 25, 2020

Coverage Status

Coverage increased (+0.004%) to 89.862% when pulling b3c4990 on gocarlos:refactor--more-cpp into ae2c5f7 on cose-wg:master.

Comment thread include/cose/cose.h
typedef struct _cose_counterSignature* HCOSE_COUNTERSIGN;
typedef struct _cose_counterSignature1* HCOSE_COUNTERSIGN1;
typedef struct _cose_key* HCOSE_KEY;
typedef class COSE_KEY* HCOSE_KEY;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My C compiler is going to produce an error over the word class in this file.

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