syslog-ng icon indicating copy to clipboard operation
syslog-ng copied to clipboard

Fix signed/unsigned comparison in `evt_tag_mem()` and add unit tests for it

Open OverOrion opened this issue 2 years ago • 5 comments

This PR adds some unit tests regarding evt_tag_mem(). There is also a fix regarding signed/unsigned comparison in evt_tag_mem().

OverOrion avatar Jul 12 '22 14:07 OverOrion

No news file has been detected. Please write one, if applicable.

github-actions[bot] avatar Jul 12 '22 14:07 github-actions[bot]

We could strip all unneeded functions of eventlog and merge it into our core library.

It was originally BSD licensed but that could be changed IMHO. Eventlog was Balabit only if I remember correctly.

But even if it wasn't it can be converted to lgpl while keeping the old bsd header.

On Tue, Jul 12, 2022, 16:48 László Várady @.***> wrote:

@.**** commented on this pull request.

In tests/unit/Makefile.am https://github.com/syslog-ng/syslog-ng/pull/4077#discussion_r919066480:

@@ -12,7 +12,8 @@ tests_unit_TESTS =
tests/unit/test_zone
tests/unit/test_pathutils
tests/unit/test_logwriter \

  • tests/unit/test_thread_wakeup
  • tests/unit/test_thread_wakeup \
  • tests/unit/test_evt_tag_mem

Thank you for the tests.

There is ongoing work about moving all the remaining unit tests from tests/unit to near to the tested unit (#4046 https://github.com/syslog-ng/syslog-ng/pull/4046).

For example, lib/eventlog/tests might be a better place.

(eventlog used to be a separate library maintained in a separate repository, but we've merged it into the syslog-ng repo after finding out that eventlog did not become too widespread. For that reason, you won't find Criterion-based tests on that directory, but I think it is absolutely okay if we add one.)

— Reply to this email directly, view it on GitHub https://github.com/syslog-ng/syslog-ng/pull/4077#pullrequestreview-1035946736, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFOK5RXWTKKL44AG7YKMVDVTWATVANCNFSM53LIA7GA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

bazsi avatar Jul 12 '22 15:07 bazsi

Thanks, @bazsi. Yes, everything under eventlog/src has a Balabit copyright notice, the author is @bazsi :) LGPL sounds good.

I guess we can relicense eventlog in a separate PR and just use an LGPL unit test here.

MrAnno avatar Jul 12 '22 15:07 MrAnno

Just checked that all commits made in this repo and the original repo was made by Balabit employee (the time of creating), plus as you mentioned @bazsi who proposed the change. There is one exception from this is the https://github.com/syslog-ng/syslog-ng/pull/2015/files, which just adds proper include headers and such should not be copyrightable.

Kokan avatar Jul 13 '22 07:07 Kokan

I think this would be an improvement wrt licensing.

On Wed, Jul 13, 2022 at 9:36 AM Kókai Péter @.***> wrote:

Just checked that all commits made in this repo and the original repo was made by Balabit employee (the time of creating), plus as you mentioned @bazsi https://github.com/bazsi who proposed the change. There is one exception from this is the https://github.com/syslog-ng/syslog-ng/pull/2015/files, which just adds proper include headers and such should not be copyrightable.

— Reply to this email directly, view it on GitHub https://github.com/syslog-ng/syslog-ng/pull/4077#issuecomment-1182871407, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFOK5UNUQ5SI6JTBAKSWJ3VTZWWXANCNFSM53LIA7GA . You are receiving this because you were mentioned.Message ID: @.***>

-- Bazsi

bazsi avatar Jul 13 '22 07:07 bazsi

Build FAILURE

kira-syslogng avatar Sep 07 '22 09:09 kira-syslogng

@kira-syslogng retest this please

MrAnno avatar Sep 07 '22 15:09 MrAnno