Skip to content
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

Add C++ plugin support and example class random-choice-generator() #4484

Merged
merged 13 commits into from
May 25, 2023

Conversation

alltilla
Copy link
Collaborator

@alltilla alltilla commented May 16, 2023

This PR adds support for creating C++ plugins

Internal API breaking changes

  • lib/templates: Public field LogTemplate::template has been renamed to LogTemplate::template_str.
  • lib/logmsg: Public field LogMessage::protected has been renamed to LogMessage::write_protected.

C++ helper headers: compat/cpp-start.h and compat/cpp-end.h

Starting your C++ header with #include "compat/cpp-start.h" and ending it with #include "compat/cpp-end.h" helps you with two things:

  • You will be able to include C headers.
  • C files will be able to include your C++ header.

Limitations

  • Obviously you can only write code in your C++ header which is syntactically correct in a C code, so you cannot define classes, etc. Do these in either your .cpp file or in a separate .hpp file, which does not get included in your C code.
  • As we are not preparing each C header to be includable in C++ code, but we are trying to include them with these helpers on the call site, we cannot be sure that any additional C header include will work. This adds a level of uncertainty to developing a plugin in C++.

If you find a C header that cannot be imported to C++ with these helpers, please try to fix those headers first. If it is not possible, extend this file with a workaround so others do not run into that issue again.

C++ plugin best practices

  • Minimize the C++ code in your plugin, if you can write and build/link something as C code, do so, for example boilerplates: parser.{c,h} and plugin.c. This minimizes the chance of including an incompatible C header.
  • Build/link the C++ code as C++ separately and link to that from your C lib. Adding -lstdc++ to LIBADD is neccessary in this case as it is automatically added while linking the C++ object itself, but not when linking to the C++ object from a C lib.
  • In your C++ code it is not possible to derive from a C class in the usual C way by adding a super field and filling its free_fn, because we do our own reference counting and freeing logic and we cannot rely on the casting trick of struct packing. You can create a struct in the usual C inheritance way and have a pointer to your C++ class in it and you can store a pointer to this struct in your C++ class as super, so you can access its fields. You can set the necessary virtual functions with wrapper functions of your real C++ functions in the ctor of the C style "class" == struct.

You can see an example usage of the C++ plugin support at: modules/examples/sources/random-choice-generator.

Build

By default C++ plugins are built if a C++ compiler is available. It is possible to disable it manually:

  • autotools: ../configure --disable-cpp
  • cmake: cmake -DENABLE_CPP=Off ..

On MacOS, the C++ support is disabled. See: #4484 (comment)

Example plugin: random-choice-generator()

It is added to demonstrate the C++ plugin support.

It chooses a random value from the values set in choices() and generates a message from it.

Minimal config:

source s_random_choice {
  random-choice-generator(choices("foo" "bar" "baz"));
};

Signed-off-by: Attila Szakacs attila.szakacs@axoflow.com

@kira-syslogng
Copy link
Contributor

Build FAILURE

@alltilla alltilla force-pushed the random-choice-generator branch 5 times, most recently from c8d8689 to 7683897 Compare May 16, 2023 16:57
typedef struct RandomChoiceGeneratorSourceDriver_ RandomChoiceGeneratorSourceDriver;

class RandomChoiceGeneratorCpp
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could derive this from the source driver directly.

Maybe we would have an issue with the destructor side (as we need to be able to free this from the c side as the ref count gets to zero).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I came to the conclusion that there would be problems on both ctor and dtor side if we derived directly, but it was a busy day, so I will think this through again tomorrow.

Copy link
Collaborator

@MrAnno MrAnno May 16, 2023

Choose a reason for hiding this comment

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

g++ and gcc pack structs differently.
I haven't checked the standard yet, but inheriting a C struct from C++ is not straight-forward as far as I remember.

Virtual functions will be a problem too (especially the destructor).

@alltilla
Copy link
Collaborator Author

macOS tries to include the VERSION file in the root dir:

In file included from modules/examples/sources/random-choice-generator/random-choice-generator.cpp:23:
In file included from /Applications/Xcode_14.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/string:532:
In file included from /Applications/Xcode_14.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/string_view:199:
./version:1:1: error: expected unqualified-id
4.2.0

The only solution I have seen is to rename the VERSION file to VERSION.txt. I don't want to do that, as there are a lot of scripts in different repos, which depend on the VERSION file. I disabled the C++ support on macOS by default.

@alltilla alltilla force-pushed the random-choice-generator branch 2 times, most recently from e694d72 to 9c9ccb0 Compare May 17, 2023 09:16
lib/compat/cpp-end.h Outdated Show resolved Hide resolved
@kira-syslogng
Copy link
Contributor

Build FAILURE

@MrAnno
Copy link
Collaborator

MrAnno commented May 18, 2023

LGTM so far, exciting 👍

alltilla added a commit to alltilla/syslog-ng that referenced this pull request May 18, 2023
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
@alltilla
Copy link
Collaborator Author

  • Rebased to master.
  • Added comments to compat/cpp-start.h and compat/cpp-end.h.
  • Added newsfiles.

@alltilla alltilla changed the title random-choice-generator: add C++ example class random-choice-generator: add C++ plugin support and example class May 18, 2023
alltilla added a commit to alltilla/syslog-ng that referenced this pull request May 18, 2023
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
@alltilla alltilla changed the title random-choice-generator: add C++ plugin support and example class Add C++ plugin support and example class random-choice-generator() May 18, 2023
alltilla added a commit to alltilla/syslog-ng that referenced this pull request May 18, 2023
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
alltilla added a commit to alltilla/syslog-ng that referenced this pull request May 18, 2023
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
@alltilla
Copy link
Collaborator Author

Added cmake support.

New commits:

  • nvtable,serialize: fix signed comparison warnings
  • cmake: add ENABLE_CPP option
  • cmake: do not enable some warnings for C++ if not available

Changes were amended to random-choice-generator: add C++ example class:

  • added a cast in the usleep() call
  • completed CMakeLists.txt

alltilla added a commit to alltilla/syslog-ng that referenced this pull request May 18, 2023
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
@alltilla alltilla marked this pull request as ready for review May 18, 2023 11:42
alltilla added a commit to alltilla/syslog-ng that referenced this pull request May 18, 2023
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
configure.ac Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
g++ drops errors for these.

Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
It conflicts with the C++ syntax.

Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
It conflicts with the C++ syntax.

Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
It conflicts with the C++ syntax.

Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
C++ compat helper headers: "compat/cpp-start.h" and "compat/cpp-end.h"

Starting your C++ header with #include "compat/cpp-start.h" and ending it
with #include "compat/cpp-end.h" helps you with two things:
  - You will be able to include C headers.
  - C files will be able to include your C++ header.

Limitations:
  - Obviously you can only write code in your C++ header which is
    syntactically correct in a C code, so you cannot define classes, etc.
    Do these in either your .cpp file or in a separate .hpp file, which
    does not get included in your C code.
  - As we are not preparing each C header to be includable in C++ code,
    but we are trying to include them with these helpers on the call site,
    we cannot be sure that any additional C header include will work.
    This adds a level of uncertainty to developing a plugin in C++.

If you find a C header that cannot be imported to C++ with these helpers,
please try to fix those headers first.  If it is not possible, extend this
file with a workaround so others do not run into that issue again.

Best practices:
  - Minimize the C++ code in your plugin, if you can write and build/link
    something as C code, do so, for example boilerplates: parser.{c,h}
    and plugin.c.  This minimizes the chance of including an incompatible
    C header.
  - Build/link the C++ code as C++ separately and link to that from your
    C lib.  Adding -lstdc++ to LIBADD is neccessary in this case as it is
    automatically added while linking the C++ object itself, but not when
    linking to the C++ object from a C lib.
  - In your C++ code it is not possible to derive from a C class in the
    usual C way by adding a super field and filling its free_fn, because
    we do our own reference counting and freeing logic and we cannot rely
    on the casting trick of struct packing.  You can create a struct
    in the usual C inheritance way and have a pointer to your C++ class
    in it and you can store a pointer to this struct in your C++ class as
    super, so you can access its fields. You can set the necessary virtual
    functions with wrapper functions of your real C++ functions in the
    ctor of the C style "class" == struct.

You can see an example usage of the C++ plugin support at:
     modules/examples/sources/random-choice-generator

Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
Signed-off-by: Attila Szakacs <attila.szakacs@axoflow.com>
@MrAnno MrAnno merged commit 61be3fc into syslog-ng:master May 25, 2023
15 checks passed
BKPepe added a commit to BKPepe/packages that referenced this pull request Sep 3, 2023
Makefile changes:
- Since version 4.3.0, there is required to use pcre2 instead of pcre
Reference: syslog-ng/syslog-ng#4537

- Disable c++ support by default to avoid picking libstdcpp dependency
Reference: syslog-ng/syslog-ng#4484

Config changes:
- Bump version in config file

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
BKPepe added a commit to openwrt/packages that referenced this pull request Sep 15, 2023
Makefile changes:
- Since version 4.3.0, there is required to use pcre2 instead of pcre
Reference: syslog-ng/syslog-ng#4537

- Disable c++ support by default to avoid picking libstdcpp dependency
Reference: syslog-ng/syslog-ng#4484

Config changes:
- Bump version in config file

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
BKPepe added a commit to openwrt/packages that referenced this pull request Sep 15, 2023
Makefile changes:
- Since version 4.3.0, there is required to use pcre2 instead of pcre
Reference: syslog-ng/syslog-ng#4537

- Disable c++ support by default to avoid picking libstdcpp dependency
Reference: syslog-ng/syslog-ng#4484

Config changes:
- Bump version in config file

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
(cherry picked from commit c43599b)
BKPepe added a commit to openwrt/packages that referenced this pull request Sep 15, 2023
Makefile changes:
- Since version 4.3.0, there is required to use pcre2 instead of pcre
Reference: syslog-ng/syslog-ng#4537

- Disable c++ support by default to avoid picking libstdcpp dependency
Reference: syslog-ng/syslog-ng#4484

Config changes:
- Bump version in config file

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
(cherry picked from commit c43599b)
BKPepe added a commit to openwrt/packages that referenced this pull request Sep 15, 2023
Makefile changes:
- Since version 4.3.0, there is required to use pcre2 instead of pcre
Reference: syslog-ng/syslog-ng#4537

- Disable c++ support by default to avoid picking libstdcpp dependency
Reference: syslog-ng/syslog-ng#4484

Config changes:
- Bump version in config file

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
(cherry picked from commit c43599b)
lu-zero pushed a commit to domo-iot/packages that referenced this pull request Oct 23, 2023
Makefile changes:
- Since version 4.3.0, there is required to use pcre2 instead of pcre
Reference: syslog-ng/syslog-ng#4537

- Disable c++ support by default to avoid picking libstdcpp dependency
Reference: syslog-ng/syslog-ng#4484

Config changes:
- Bump version in config file

Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
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.

None yet

4 participants