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.

Sign up for GitHub

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

Jump to bottom

Support FreeBSD 14.0 + NetBSD 10.0 + add testing #74

Merged
merged 10 commits into from
Jun 6, 2024
Merged

Support FreeBSD 14.0 + NetBSD 10.0 + add testing #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6247204
Try building something with FreeBSD 14.0 qemu action
GreyCat May 27, 2024
bb1a6f9
Try actual build with cmake
GreyCat May 27, 2024
6a44911
Add googletest for installation
GreyCat May 27, 2024
bf75eb9
Add NetBSD 10.0 run
GreyCat Jun 3, 2024
6e0ece3
NetBSD build: remove extra debug print, add compiler comment
GreyCat Jun 3, 2024
17b3a25
Attempt to work around NetBSD build problem
GreyCat Jun 3, 2024
c3bae0f
Another try to detect BSD properly
GreyCat Jun 3, 2024
26a141a
Try NetBSD with __BYTE_ORDER / __BIG_ENDIAN / __LITTLE_ENDIAN defines
GreyCat Jun 3, 2024
44bce3e
Add workaround for FreeBSD+NetBSD CITRUS iconv, skipping test
GreyCat Jun 5, 2024
7b4c29a
Fix sys/param.h problem on Windows
GreyCat Jun 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,57 @@ jobs:
run: .build\build.ps1 -GTestPath C:\vcpkg\installed\x64-windows
- name: unittest
run: .build\run-unittest.ps1
freebsd_14_0:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Run in FreeBSD
uses: vmactions/freebsd-vm@v1
with:
release: "14.0"
usesh: true
prepare: |
pkg install -y cmake googletest

run: |
echo '#### System information'
whoami
env
freebsd-version
c++ --version
gcc --version
clang++ --version
pkg info

echo '#### Building'
.build/build -DCMAKE_CXX_STANDARD=11 -DCMAKE_CXX_STANDARD_REQUIRED=ON -DCMAKE_CXX_EXTENSIONS=OFF

echo '#### Testing'
.build/run-unittest
netbsd_10_0:
runs-on: ubuntu-latest
# Doesn't use clang, has gcc:
# c++ (nb3 20231008) 10.5.0
# Copyright (C) 2020 Free Software Foundation, Inc.
steps:
- uses: actions/checkout@v4
- name: Run in NetBSD
uses: vmactions/netbsd-vm@v1
with:
release: "10.0"
usesh: true
prepare: |
/usr/sbin/pkg_add cmake googletest

run: |
echo '#### System information'
whoami
env
c++ --version
gcc --version

echo '#### Building'
.build/build -DCMAKE_CXX_STANDARD=11 -DCMAKE_CXX_STANDARD_REQUIRED=ON -DCMAKE_CXX_EXTENSIONS=OFF

echo '#### Testing'
.build/run-unittest
17 changes: 16 additions & 1 deletion kaitai/kaitaistream.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,25 @@
#define __BYTE_ORDER BYTE_ORDER
#define __BIG_ENDIAN BIG_ENDIAN
#define __LITTLE_ENDIAN LITTLE_ENDIAN
#else // !__APPLE__ or !_MSC_VER or !__QNX__
#else
// At this point it's either Linux or BSD. Both have "sys/param.h", so it's safe to include
#include <sys/param.h>
#if defined(BSD)
// Supposed to work on FreeBSD: https://man.freebsd.org/cgi/man.cgi?query=bswap16&manpath=FreeBSD+14.0-RELEASE
// Supposed to work on NetBSD: https://man.netbsd.org/NetBSD-10.0/bswap16.3
#include <sys/endian.h>
#include <sys/types.h>
#define bswap_16(x) bswap16(x)
#define bswap_32(x) bswap32(x)
#define bswap_64(x) bswap64(x)
#define __BYTE_ORDER BYTE_ORDER
#define __BIG_ENDIAN BIG_ENDIAN
#define __LITTLE_ENDIAN LITTLE_ENDIAN
#else // !__APPLE__ or !_MSC_VER or !__QNX__ or !BSD
#include <endian.h>
#include <byteswap.h>
#endif
#endif

#include <cstring> // std::memcpy
#include <iostream>
Expand Down
11 changes: 10 additions & 1 deletion tests/unittest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
#include <gtest/gtest.h>
#endif

// Necessary for proper detection of *BSD
#if !defined(_MSC_VER)
#include <sys/param.h>
#endif
Comment on lines +7 to +10
Copy link
Member

Choose a reason for hiding this comment

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

@GreyCat I suspect this isn't actually necessary since you're only using the __FreeBSD__ and __NetBSD__ macros, not the BSD macro which indeed requires #include <sys/param.h>: https://sourceforge.net/p/predef/wiki/OperatingSystems/#bsd-environment

I think this unnecessary include should be removed. For now, I guess this condition is fine, because the test exclusions should be minimal and these are the only BSD variants that we know are problematic:

kaitai_struct_cpp_stl_runtime/tests/unittest.cpp

Line 383 in 7b4c29a

#if !defined(__FreeBSD__) && !defined(__NetBSD__)

Copy link
Member Author

Choose a reason for hiding this comment

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

I recall in my testing that it actually required that include, but maybe I've indeed got something crossed, so if you have an idea what we can eliminate/optimize here — absolutely feel free to go ahead and change :)

Copy link
Member

Choose a reason for hiding this comment

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

I think (at least I haven't seen any single reference on the internet that would suggest otherwise) that sys/param.h only defines BSD (see FreeBSD's sys/param.h, NetBSD's sys/param.h), whereas __FreeBSD__ and __NetBSD__ are predefined macros by the compiler, so you don't need to include anything to use them.


#include "kaitai/kaitaistream.h"
#include "kaitai/exceptions.h"
#include <sstream>
Expand Down Expand Up @@ -359,7 +364,6 @@ TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_euc_jp_too_short)
}
}


TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_gb2312_too_short)
{
try {
Expand All @@ -376,8 +380,12 @@ TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_gb2312_too_short)
}
}

#if !defined(__FreeBSD__) && !defined(__NetBSD__)
TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_gb2312_two_bytes)
{
// 0xB0 0x30 is illegal sequence in GB2312: 0xB0 must be followed by [0xA1..0xFE].
// However, some iconv engines, namely CITRUS integrated with modern FreeBSD (10+) and NetBSD,
// are not considering this as error and thus not returning EILSEQ.
try {
std::string res = kaitai::kstream::bytes_to_str("\xb0\x30", "GB2312");
FAIL() << "Expected illegal_seq_in_encoding exception";
Expand All @@ -391,6 +399,7 @@ TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_gb2312_two_bytes)
#endif
}
}
#endif

TEST(KaitaiStreamTest, bytes_to_str_invalid_seq_utf_16le_odd_bytes)
{
Expand Down