diff --git a/.github/workflows/cpp.yml b/.github/workflows/cpp.yml index 7cf8109698..7841249478 100644 --- a/.github/workflows/cpp.yml +++ b/.github/workflows/cpp.yml @@ -23,7 +23,7 @@ on: - 'main' env: - APT_PACKAGES: libspdlog-dev libpcap-dev libevdev2 libev-dev protobuf-compiler libgtest-dev libgmock-dev + APT_PACKAGES: libspdlog-dev libpcap-dev libevdev2 libev-dev protobuf-compiler libgtest-dev libgmock-dev meson ninja-build jobs: unit_tests: @@ -41,7 +41,7 @@ jobs: run: DEBUG=1 make -j $(nproc) test - name: Run unit tests - run: (set -o pipefail && bin/piscsi_test | tee piscsi_test_log.txt) + run: (set -o pipefail && GTEST_SHUFFLE=1 bin/piscsi_test | tee piscsi_test_log.txt) - name: Upload logs uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 @@ -49,6 +49,32 @@ jobs: name: piscsi_test_log.txt path: cpp/piscsi_test_log.txt + unit_tests_meson: + runs-on: ubuntu-24.04 + defaults: + run: + working-directory: . + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 + + - name: Install dependencies + run: sudo apt-get install ${{ env.APT_PACKAGES }} + + - name: Configure build + run: meson setup build + + - name: Build unit tests + run: meson compile -C build + + - name: Run unit tests + run: GTEST_SHUFFLE=1 meson test -C build + + - name: Upload logs + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 + with: + name: testlog.txt + path: build/meson-logs/testlog.txt + sonarcloud: runs-on: ubuntu-24.04 if: > diff --git a/cpp/.gitignore b/cpp/.gitignore index 6f970d2d70..32aee638ca 100644 --- a/cpp/.gitignore +++ b/cpp/.gitignore @@ -11,4 +11,3 @@ piscsi.dat obj bin coverage -generated diff --git a/cpp/devices/scsi_command_util.cpp b/cpp/devices/scsi_command_util.cpp index 98016af199..94d7f94f5e 100644 --- a/cpp/devices/scsi_command_util.cpp +++ b/cpp/devices/scsi_command_util.cpp @@ -44,12 +44,22 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span 0) { + // Check if we can read the page code and length + if (offset >= static_cast(buf.size())) { + break; + } + // Format device page if (const int page = buf[offset]; page == 0x03) { if (length < 14) { throw scsi_exception(sense_key::illegal_request, asc::invalid_field_in_parameter_list); } + // Check if we can read the sector size field at offset + 12 + if (offset + 13 >= static_cast(buf.size())) { + break; + } + // With this page the sector size for a subsequent FORMAT can be selected, but only very few // drives support this, e.g FUJITSU M2624S // We are fine as long as the current sector size remains unchanged @@ -69,6 +79,10 @@ string scsi_command_util::ModeSelect(scsi_command cmd, cdb_t cdb, span= static_cast(buf.size())) { + break; + } + const int size = buf[offset + 1] + 2; length -= size; diff --git a/cpp/generated/.gitignore b/cpp/generated/.gitignore new file mode 100644 index 0000000000..a130dc1715 --- /dev/null +++ b/cpp/generated/.gitignore @@ -0,0 +1,3 @@ +piscsi_interface.pb.h +piscsi_interface.pb.cc +piscsi_interface.pb.cpp diff --git a/cpp/generated/meson.build b/cpp/generated/meson.build new file mode 100644 index 0000000000..c00a616be4 --- /dev/null +++ b/cpp/generated/meson.build @@ -0,0 +1,13 @@ +piscsi_interface_src = custom_target( + 'piscsi_interface_proto', + input : meson.project_source_root() / 'cpp' / 'piscsi_interface.proto', + output : ['piscsi_interface.pb.cc', 'piscsi_interface.pb.h'], + command : [ + protoc, + '--cpp_out=@BUILD_ROOT@/cpp/generated', + '--proto_path=' + meson.project_source_root() / 'cpp', + '@INPUT@' + ], + build_by_default : true, + install : false +) diff --git a/cpp/meson.build b/cpp/meson.build new file mode 100644 index 0000000000..3303a51e6f --- /dev/null +++ b/cpp/meson.build @@ -0,0 +1,180 @@ +controllers_src = [ + 'controllers/abstract_controller.cpp', + 'controllers/controller_manager.cpp', + 'controllers/phase_handler.cpp', + 'controllers/scsi_controller.cpp', +] +devices_src = [ + 'devices/cd_track.cpp', + 'devices/ctapdriver.cpp', + 'devices/device_factory.cpp', + 'devices/device_logger.cpp', + 'devices/device.cpp', + 'devices/disk_cache.cpp', + 'devices/disk_track.cpp', + 'devices/disk.cpp', + 'devices/host_services.cpp', + 'devices/mode_page_device.cpp', + 'devices/primary_device.cpp', + 'devices/scsi_command_util.cpp', + 'devices/scsi_daynaport.cpp', + 'devices/scsi_printer.cpp', + 'devices/scsi_streamer.cpp', + 'devices/scsicd.cpp', + 'devices/scsihd_nec.cpp', + 'devices/scsihd.cpp', + 'devices/scsimo.cpp', + 'devices/storage_device.cpp', +] +hal_src = [ + 'hal/bus.cpp', + 'hal/data_sample.cpp', + 'hal/gpiobus_factory.cpp', + 'hal/gpiobus_raspberry.cpp', + 'hal/gpiobus_virtual.cpp', + 'hal/gpiobus.cpp', + 'hal/sbc_version.cpp', + 'hal/systimer_raspberry.cpp', + 'hal/systimer.cpp', +] +piscsi_core_src = [ + 'piscsi/command_context.cpp', + 'piscsi/localizer.cpp', + 'piscsi/piscsi_core.cpp', + 'piscsi/piscsi_executor.cpp', + 'piscsi/piscsi_image.cpp', + 'piscsi/piscsi_response.cpp', + 'piscsi/piscsi_service.cpp', +] + controllers_src + devices_src + hal_src +piscsi_src = [ + 'piscsi/piscsi.cpp', +] +scsictl_core_src = [ + 'scsictl/scsictl_commands.cpp', + 'scsictl/scsictl_core.cpp', + 'scsictl/scsictl_display.cpp', + 'scsictl/scsictl_parser.cpp', +] +scsictl_src = [ + 'scsictl/scsictl.cpp', +] +scsidump_core_src = [ + 'scsidump/scsidump_core.cpp', +] +scsidump_src = [ + 'scsidump/scsidump.cpp', +] + scsidump_core_src + hal_src +scsiloop_src = [ + 'scsiloop/scsiloop.cpp', + 'scsiloop/scsiloop_core.cpp', + 'scsiloop/scsiloop_cout.cpp', + 'scsiloop/scsiloop_gpio.cpp', + 'scsiloop/scsiloop_timer.cpp', +] + hal_src +scsimon_src = [ + 'scsimon/scsimon.cpp', + 'scsimon/sm_core.cpp', + 'scsimon/sm_html_report.cpp', + 'scsimon/sm_json_report.cpp', + 'scsimon/sm_vcd_report.cpp', +] + hal_src +shared_src = [ + 'shared/network_util.cpp', + 'shared/piscsi_util.cpp', + 'shared/piscsi_version.cpp', +] +protobuf_src = [ + 'shared/protobuf_util.cpp', +] +test_src = [ + 'test/abstract_controller_test.cpp', + 'test/bus_test.cpp', + 'test/command_context_test.cpp', + 'test/controller_manager_test.cpp', + 'test/ctapdriver_test.cpp', + 'test/device_factory_test.cpp', + 'test/device_test.cpp', + 'test/disk_test.cpp', + 'test/gpiobus_raspberry_test.cpp', + 'test/host_services_test.cpp', + 'test/linux_os_stubs.cpp', + 'test/localizer_test.cpp', + 'test/mode_page_device_test.cpp', + 'test/network_util_test.cpp', + 'test/phase_handler_test.cpp', + 'test/piscsi_exceptions_test.cpp', + 'test/piscsi_executor_test.cpp', + 'test/piscsi_image_test.cpp', + 'test/piscsi_response_test.cpp', + 'test/piscsi_service_test.cpp', + 'test/piscsi_util_test.cpp', + 'test/primary_device_test.cpp', + 'test/protobuf_util_test.cpp', + 'test/scsi_command_util_test.cpp', + 'test/scsi_controller_test.cpp', + 'test/scsi_daynaport_test.cpp', + 'test/scsi_printer_test.cpp', + 'test/scsicd_test.cpp', + 'test/scsictl_commands_test.cpp', + 'test/scsictl_display_test.cpp', + 'test/scsictl_parser_test.cpp', + 'test/scsidump_test.cpp', + 'test/scsihd_nec_test.cpp', + 'test/scsihd_test.cpp', + 'test/scsimo_test.cpp', + 'test/storage_device_test.cpp', + 'test/test_setup.cpp', + 'test/test_shared.cpp', +] + scsidump_core_src + +subdir('generated') + +if to_build.contains('piscsi') + piscsi_bin = executable('piscsi', + piscsi_core_src + piscsi_src + shared_src + protobuf_src + piscsi_interface_src[0], + dependencies : [pthread_dep, pcap_dep, protobuf_dep], + install : true, + ) +endif + +if to_build.contains('scsictl') + scsictl_bin = executable('scsictl', + scsictl_core_src + scsictl_src + shared_src + protobuf_src + piscsi_interface_src[0], + dependencies : [pthread_dep, protobuf_dep], + install : true, + ) +endif + +if to_build.contains('scsimon') + scsimon_bin = executable('scsimon', + scsimon_src + shared_src, + dependencies : [pthread_dep], + install : true, + ) +endif + +if to_build.contains('scsiloop') + scsiloop_bin = executable('scsiloop', + scsiloop_src + shared_src, + dependencies : [pthread_dep], + install : true, + ) +endif + +if connect_type == 'FULLSPEC' and to_build.contains('scsidump') + scsidump_bin = executable('scsidump', + scsidump_src + shared_src, + dependencies : [pthread_dep], + install : true, + ) +endif + +if gtest_dep.found() and gmock_dep.found() and to_build.contains('test') + test_bin = executable('piscsi_test', + piscsi_core_src + scsictl_core_src + test_src + shared_src + protobuf_src + piscsi_interface_src[0], + dependencies : [pthread_dep, pcap_dep, protobuf_dep, gtest_dep, gmock_dep], + link_args : ['-Wl,--wrap=fopen64'], + install : false + ) + test('unit', test_bin) +endif diff --git a/cpp/piscsi/piscsi_executor.cpp b/cpp/piscsi/piscsi_executor.cpp index ace9cb2b21..4a4878112f 100644 --- a/cpp/piscsi/piscsi_executor.cpp +++ b/cpp/piscsi/piscsi_executor.cpp @@ -41,6 +41,12 @@ bool PiscsiExecutor::ProcessDeviceCmd(const CommandContext& context, const PbDev auto device = controller_manager.GetDeviceForIdAndLun(id, lun); + // Protect against non-existing device + if (!device) { + spdlog::error("ProcessDeviceCmd: No device found for id={}, lun={}", id, lun); + return false; + } + if (!ValidateOperationAgainstDevice(context, *device, operation)) { return false; } diff --git a/cpp/scsictl/scsictl_parser.cpp b/cpp/scsictl/scsictl_parser.cpp index 175b5ba854..eee4784704 100644 --- a/cpp/scsictl/scsictl_parser.cpp +++ b/cpp/scsictl/scsictl_parser.cpp @@ -11,6 +11,9 @@ PbOperation ScsictlParser::ParseOperation(string_view operation) const { + if (operation.empty()) { + return NO_OPERATION; + } const auto& it = operations.find(tolower(operation[0])); return it != operations.end() ? it->second : NO_OPERATION; } diff --git a/cpp/test/scsihd_test.cpp b/cpp/test/scsihd_test.cpp index 693f450bfe..568f407585 100644 --- a/cpp/test/scsihd_test.cpp +++ b/cpp/test/scsihd_test.cpp @@ -113,15 +113,20 @@ TEST(ScsiHdTest, ModeSelect) cmd[1] = 0x10; // Page 3 (Device Format Page) buf[4] = 0x03; - // 512 bytes per sector + // 22 bytes (standard format page length) + buf[5] = 0x16; + // 512 bytes per sector (offset 12 from page start = offset 16 from buffer start) buf[16] = 0x02; EXPECT_NO_THROW(hd.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255)) << "MODE SELECT(6) is supported"; buf[4] = 0; + buf[5] = 0; buf[16] = 0; // Page 3 (Device Format Page) buf[8] = 0x03; - // 512 bytes per sector + // 22 bytes (standard format page length) + buf[9] = 0x16; + // 512 bytes per sector (offset 12 from page start = offset 20 from buffer start) buf[20] = 0x02; EXPECT_NO_THROW(hd.ModeSelect(scsi_command::eCmdModeSelect10, cmd, buf, 255)) << "MODE SELECT(10) is supported"; } diff --git a/cpp/test/scsimo_test.cpp b/cpp/test/scsimo_test.cpp index d38ee4a42b..b3fcb14176 100644 --- a/cpp/test/scsimo_test.cpp +++ b/cpp/test/scsimo_test.cpp @@ -145,14 +145,19 @@ TEST(ScsiMoTest, ModeSelect) cmd[1] = 0x10; // Page 3 (Device Format Page) buf[4] = 0x03; + // 22 bytes (standard format page length) + buf[5] = 0x16; // 2048 bytes per sector buf[16] = 0x08; EXPECT_NO_THROW(mo.ModeSelect(scsi_command::eCmdModeSelect6, cmd, buf, 255)) << "MODE SELECT(6) is supported"; buf[4] = 0; + buf[5] = 0; buf[16] = 0; // Page 3 (Device Format Page) buf[8] = 0x03; + // 22 bytes (standard format page length) + buf[9] = 0x16; // 2048 bytes per sector buf[20] = 0x08; EXPECT_NO_THROW(mo.ModeSelect(scsi_command::eCmdModeSelect10, cmd, buf, 255)) << "MODE SELECT(10) is supported"; diff --git a/cpp/test/storage_device_test.cpp b/cpp/test/storage_device_test.cpp index 033766f4c3..7897092e1c 100644 --- a/cpp/test/storage_device_test.cpp +++ b/cpp/test/storage_device_test.cpp @@ -15,7 +15,20 @@ using namespace filesystem; -TEST(StorageDeviceTest, SetGetFilename) +class StorageDeviceTest : public ::testing::Test { +protected: + void SetUp() override { + // Clean up any previous test state before each test + StorageDevice::UnreserveAll(); + } + + void TearDown() override { + // Clean up after each test + StorageDevice::UnreserveAll(); + } +}; + +TEST_F(StorageDeviceTest, SetGetFilename) { MockStorageDevice device; @@ -23,7 +36,7 @@ TEST(StorageDeviceTest, SetGetFilename) EXPECT_EQ("filename", device.GetFilename()); } -TEST(StorageDeviceTest, ValidateFile) +TEST_F(StorageDeviceTest, ValidateFile) { MockStorageDevice device; @@ -59,7 +72,7 @@ TEST(StorageDeviceTest, ValidateFile) remove(filename); } -TEST(StorageDeviceTest, MediumChanged) +TEST_F(StorageDeviceTest, MediumChanged) { MockStorageDevice device; @@ -72,7 +85,7 @@ TEST(StorageDeviceTest, MediumChanged) EXPECT_FALSE(device.IsMediumChanged()); } -TEST(StorageDeviceTest, GetIdsForReservedFile) +TEST_F(StorageDeviceTest, GetIdsForReservedFile) { const int ID = 1; const int LUN = 0; @@ -81,7 +94,6 @@ TEST(StorageDeviceTest, GetIdsForReservedFile) MockAbstractController controller(bus, ID); auto device = make_shared(LUN); device->SetFilename("filename"); - StorageDevice::UnreserveAll(); EXPECT_TRUE(controller_manager.AttachToController(*bus, ID, device)); @@ -100,7 +112,7 @@ TEST(StorageDeviceTest, GetIdsForReservedFile) EXPECT_EQ(-1, lun3); } -TEST(StorageDeviceTest, UnreserveAll) +TEST_F(StorageDeviceTest, UnreserveAll) { const int ID = 1; const int LUN = 0; @@ -119,7 +131,7 @@ TEST(StorageDeviceTest, UnreserveAll) EXPECT_EQ(-1, lun); } -TEST(StorageDeviceTest, GetSetReservedFiles) +TEST_F(StorageDeviceTest, GetSetReservedFiles) { const int ID = 1; const int LUN = 0; @@ -142,13 +154,13 @@ TEST(StorageDeviceTest, GetSetReservedFiles) EXPECT_TRUE(reserved_files.contains("filename")); } -TEST(StorageDeviceTest, FileExists) +TEST_F(StorageDeviceTest, FileExists) { EXPECT_FALSE(StorageDevice::FileExists("/non_existing_file")); EXPECT_TRUE(StorageDevice::FileExists("/dev/null")); } -TEST(StorageDeviceTest, GetFileSize) +TEST_F(StorageDeviceTest, GetFileSize) { MockStorageDevice device; diff --git a/doc/meson.build b/doc/meson.build new file mode 100644 index 0000000000..7ca59bf951 --- /dev/null +++ b/doc/meson.build @@ -0,0 +1,40 @@ +man_pages = [ + 'piscsi.1', +] + +if to_build.contains('scsictl') + man_pages += 'scsictl.1' +endif + +if to_build.contains('scsimon') + man_pages += 'scsimon.1' +endif + +if to_build.contains('scsidump') + man_pages += 'scsidump.1' +endif + +if to_build.contains('scsiloop') + man_pages += 'scsiloop.1' +endif + +foreach man : man_pages + install_data(man, install_dir : get_option('prefix') / get_option('mandir') / 'man1') + # Also generate a plain text version and install it into the doc/ directory in the source tree + txt = man.split('.')[0] + '_man_page.txt' + custom_target( + txt, + input : man, + output : txt, + command : [ + 'sh', '-c', + 'echo "!! ------ THIS FILE IS AUTO_GENERATED! DO NOT MANUALLY UPDATE!!!" && ' + + 'echo "!! ------ The source file is $(basename @INPUT@)" && ' + + 'echo "" && ' + + 'man -l @INPUT@ | col -bx' + ], + capture: true, + install : true, + install_dir : meson.project_source_root() / 'doc', + ) +endforeach diff --git a/meson.build b/meson.build new file mode 100644 index 0000000000..5961286113 --- /dev/null +++ b/meson.build @@ -0,0 +1,39 @@ +project( + 'piscsi', + 'cpp', + version : '24.4.1', + default_options : ['cpp_std=c++20', 'warning_level=2'] +) + +common_flags = [ + '-D_FILE_OFFSET_BITS=64', + '-DFMT_HEADER_ONLY', + '-DSPDLOG_FMT_EXTERNAL', +] + +if get_option('buildtype') == 'debug' + common_flags += '-DDEBUG' +else + common_flags += '-DNDEBUG' +endif + +if host_machine.system() == 'linux' + common_flags += '-Wno-psabi' +endif + +to_build = get_option('to_build') +connect_type = get_option('connect_type') +common_flags += '-DCONNECT_TYPE_' + connect_type +add_project_arguments(common_flags, language : 'cpp') + +# Dependencies +protobuf_dep = dependency('protobuf', required : true) +protoc = find_program('protoc', required : true) +pthread_dep = dependency('threads', required : true) +pcap_dep = dependency('pcap', required : true) +gtest_dep = dependency('gtest', required : false) +gmock_dep = dependency('gmock', required : false) + +subdir('cpp') +subdir('doc') +subdir('os_integration') diff --git a/meson_options.txt b/meson_options.txt new file mode 100644 index 0000000000..72ed951740 --- /dev/null +++ b/meson_options.txt @@ -0,0 +1,14 @@ +option( + 'connect_type', + type : 'combo', + choices : ['AIBOM', 'FULLSPEC', 'GAMERNIUM', 'STANDARD'], + value : 'FULLSPEC', + description : 'PiSCSI board type' +) +option( + 'to_build', + type : 'array', + choices : ['piscsi', 'scsictl', 'scsimon', 'scsiloop', 'scsidump', 'test'], + value : ['piscsi', 'scsictl', 'scsimon', 'scsiloop', 'scsidump', 'test'], + description : 'Binaries to build' +) diff --git a/os_integration/meson.build b/os_integration/meson.build new file mode 100644 index 0000000000..b9b6278fe2 --- /dev/null +++ b/os_integration/meson.build @@ -0,0 +1,2 @@ +install_data('piscsi.conf', install_dir : 'etc' / 'rsyslog.d') +install_data('piscsi.service', install_dir : 'etc' / 'systemd' / 'system')