A few issues in cdi_test app

[f2bo]

I recently ported the cdi_test app to .NET and C# and in doing so, noticed a few (minor) issues that might require fixing. Nothing critical, but I thought I would bring them to your attention given the upcoming release (2.4.1).

• The app crashes if you press the CTRL + C key combination in Windows

  The issue is caused by the SIGINT handler which should not be registered under Windows.

  https://github.com/aws/aws-cdi-sdk/blob/eba76e328adebf2bfdf2ebdc9d0ef76e074eda29/src/test/cdi_test.c#L205-L223

  Instead, the following handles both CTRL+C and CTRL+Break correctly.

  static bool SetupSignalHandlers(void) {
    if (!CdiOsCritSectionCreate(&signal_handler_lock)) {
        return false;
    }
  
    CdiOsSignalHandlerSet(SIGSEGV, SignalHandler); // Handle segfaults
    CdiOsSignalHandlerSet(SIGABRT, SignalHandler); // Handle asserts
    CdiOsSignalHandlerSet(SIGILL, SignalHandler);  // Handle illegal instruction
    CdiOsSignalHandlerSet(SIGFPE, SignalHandler);  // Handle floating point error
  #ifdef _WIN32
    if (!SetConsoleCtrlHandler((PHANDLER_ROUTINE)Win32CtrlHandler, true)) {
        assert(0);
    }
  #else
    CdiOsSignalHandlerSet(SIGINT, SignalHandler);
  #endif
    return true;
  }

• The following check seems wrong and always prints a message, even when the --log argument is specified. https://github.com/aws/aws-cdi-sdk/blob/eba76e328adebf2bfdf2ebdc9d0ef76e074eda29/src/test/test_args.c#L950-L952

  It should probably be:

  if (*GetGlobalTestSettings()->base_log_filename_str == '\0') {

• The following conditional appears to be missing some code: https://github.com/aws/aws-cdi-sdk/blob/eba76e328adebf2bfdf2ebdc9d0ef76e074eda29/src/test/optarg.c#L74-L75

  I assume the following was meant:

  TestConsoleLog(kLogDebug, "Comparing [%s] with [%s]", type == kOptLong ?
                 opt_array_ptr[i].long_name_str : opt_array_ptr[i].short_name_str, opt_str);

[f2bo]

There also seems to be a problem with the following command lines taken from the USER_TEST_GUIDE_APP under Windows whenever the num_transactions argument is increased beyond the file size, even though the guide states:

As long as the RIFF file ends on a completed chunk, it loops back to the beginning of the file.

Receiver:

./build/debug/bin/cdi_test --adapter EFA --local_ip <rx-ipv4> -X --rx AVM --dest_port 2000 --rate 60 --num_transactions 3000 -S --id 1 --payload_size 1024 --riff --file_write ancillary_data_out.cdi --avm_anc

Transmitter:

./build/debug/bin/cdi_test --adapter EFA --local_ip <tx-ipv4> -X --tx AVM --remote_ip <rx-ipv4> --dest_port 2000 --rate 60 --num_transactions 3000 --buffer_type LINEAR -S --id 1 --payload_size 1024 --riff --file_read <Test Content Directory>/ancillary_data.cdi --avm_anc

When the number of transactions exceeds the file size, the test fails:

[2022-10-28T13:21:56-03:00] ERROR: [CdiOsRead:942] ReadFile failed. Byte Count[8]. Bytes Read[0]
[2022-10-28T13:21:56-03:00] ERROR: [GetNextRiffChunkSize:501] Failed to read chunk header from file [...\data\ancillary_data.cdi]. Read [0] header bytes.
[2022-10-28T13:21:56-03:00] ERROR: [GetNextRiffChunkSize:508] RIFF File [...\data\ancillary_data.cdi] subchunk ID is not 'ANC '.

The code to loop back to the beginning of the file expects return_val to be true when the end of the file is reached and zero bytes are read. https://github.com/aws/aws-cdi-sdk/blob/eba76e328adebf2bfdf2ebdc9d0ef76e074eda29/src/test/riff.c#L489-L497

However, under Windows, CdiOsRead returns false at EOF.

https://github.com/aws/aws-cdi-sdk/blob/eba76e328adebf2bfdf2ebdc9d0ef76e074eda29/src/common/src/os_windows.c#L938-L943

It should probably be:

        status = ReadFile(file_handle, buffer_ptr, byte_count, &bytes_read, NULL);

        if (!status || ((bytes_read > 0) && (bytes_read != byte_count))) {
            ERROR_MESSAGE("ReadFile failed. Byte Count[%d]. Bytes Read[%d]", byte_count, bytes_read);
            return_val = false;
        }

[mhhen]

Thank you for providing proposed solutions to the issues you identified. We will add the changes to the upcoming release which will be posted soon.

[f2bo]

Good. I also identified an issue in the RIFF support that results in an assertion Expression: c >= -1 && c <= 255 when running under Windows (compiled with VS2022). It can be reproduced with the following command line, where ancillary_data.cdi is the test file provided at https://cdi.elemental.com/test_content/index.html.

dump_riff /path_to_test_files/ancillary_data.cdi

It is raised when executing the following code: https://github.com/aws/aws-cdi-sdk/blob/eba76e328adebf2bfdf2ebdc9d0ef76e074eda29/src/test/riff.c#L135

And my workaround was:

print_buffer_ptr[i++] = isprint((unsigned char)data_ptr[j]) ? data_ptr[j] : '.';

[mhhen]

We will add this change as well. Thanks!