4

I have a snip of the following code which should read the first 4 objects in a .wav file in order to eventually parse the header of the file. I know I'm doing something wrong here because the buffer always passes "RIFF" without printing out Riff is found

How should I use the Switch-case in order to find the correct array characters?

#include <iostream>
#include <string>
using namespace std;

int main()
{
    
    cout << "Nom: ";
    string filename;
    cout << "First Input filename:" << endl;
    cin >> filename;
#pragma warning (disable : 4996)
    FILE* InFile = fopen(filename.c_str(), "rb");        // Open wave file in read mode

      char Buffer[4];
    while (InFile) {
        fread(Buffer, sizeof Buffer[0], 4, InFile);
        switch (Buffer[4]) {
        case 'R' +'I'+'F'+'F':
            cout << "Riff is found " << endl;

        case 'c' +'r'+ 'i'+ 'f' :
            cout << "CRiff is found " << endl;
        }
    }
}
Omar
  • 164
  • 10
  • In "switch (Buffer[4])", is Buffer[4] a char? if yes, is 'R' +'I'+'F'+'F' a char as well? I think it is wrong. – Mojtaba Valizadeh Nov 24 '21 at 18:43
  • Using `fopen` in C++ smells a bit. Why not use the tools provided in the `fstream` header? – Chris Nov 24 '21 at 18:47
  • `while (InFile)` is completely wrong. In your example, if the file was opened, it will *never* be false and the loop goes on forever. Use the return value from `fread` to decide if it was successful or not. I know this may be pseudocode, but you also have no `break` after each case so if it worked each case after the one that matched would also execute. – Retired Ninja Nov 24 '21 at 18:47
  • 1
    `switch (Buffer[4]) {` trys to compare the 5th element of a 4 element (out of bounds / undefined behavior) char array to the sum of several character values (which will overflow the char). – drescherjm Nov 24 '21 at 18:49
  • @RetiredNinja why ? it only checks if file was opened successfully as if it is not, InFile =NULL – Omar Nov 24 '21 at 18:50
  • Use `if (strncmp(Buffer, "RIFF", 4) == 0) { ... } else if (strncmp(Buffer, "crif", 4) == 0) { ... }` instead. – trojanfoe Nov 24 '21 at 18:50
  • Does this answer your question? [Why the switch statement cannot be applied on strings?](https://stackoverflow.com/questions/650162/why-the-switch-statement-cannot-be-applied-on-strings) – OrenIshShalom Nov 24 '21 at 19:15
  • 1
    @OrenIshShalom: Although your proposed duplicate is an interesting link that is relevant to the question, in my opinion, it is not a duplicate. The proposed duplicate is about **why** `switch` cannot be used with strings. However, this question is not about **why**, but rather about how to solve a specific problem. – Andreas Wenzel Nov 24 '21 at 19:23
  • 2
    FYI, in your `case` statements, the letters are converted, at compile time, to numbers. The **numbers are added together**. In other words: `R` (0x52) + `I` (0x49) + `F` (0x46) + `F` (0x46) == 0x127 == 295. So instead of comparing against multiple characters, the `case` is looking for 295; e.g. `case 295:`. – Thomas Matthews Nov 24 '21 at 19:29

3 Answers3

4

In contrast to languages such as C#, you cannot use strings in a switch expression. You will have to use if...else statements in conjunction with std::memcmp instead:

if      ( std::memcmp( Buffer, "RIFF", 4 ) == 0 )
    std::cout << "Riff is found.\n";
else if ( std::memcmp( Buffer, "crif", 4 ) == 0 )
    std::cout << "CRiff is found.\n";

Note that std::memcmp should only be used if you are sure that all character sequences are of the same length. Otherwise, it is safer to use std::strncmp.

In your posted code, what is actually happening is the following:

The expression 'R'+'I'+'F'+'F' will evaluate to the sum of the individual character codes of these letters. This is not what you want.

Andreas Wenzel
  • 22,760
  • 4
  • 24
  • 39
4

you can't compare multiple characters in a single switch.

But given that RIFF uses four character codes you can make it work with a switch statement by reading it as an int value and comparing against a multi-char literal (this is technically implementation-defined, but it'll work in all major compilers (gcc, clang, msvc))

uint32_t fourcc;
while (InFile) {
    fread(&fourcc, 4, 1, InFile);
    switch (fourcc) {
        case 'RIFF':
            std::cout << "Riff is found " << std::endl;
            break;

        case 'crif':
            std::cout << "CRiff is found " << std::endl;
            break;
    }
}

this however is not very portable and will cause problems if you need it to work on architectures with different endianness.

If you want to use a switch but not rely on implementation-defined behaviour you could e.g. use a custom string literal for this:

using fourcc = uint32_t;

struct fourcc_str {
    const char data[4];

    template<std::size_t N>
    consteval fourcc_str(const char (&str)[N]) noexcept : data{str[0], str[1], str[2], str[3]} {
        static_assert(N == 5, "A four character literal needs four characters!");
    }
};

template<fourcc_str str>
consteval fourcc operator "" _fourcc() {
    return std::bit_cast<fourcc>(str.data);
}


// then you can use it like this:
fourcc mark;
while (InFile) {
    fread(&mark, sizeof(fourcc), 1, InFile);
    switch (mark) {
        case "RIFF"_fourcc:
            std::cout << "Riff is found " << std::endl;
            break;

        case "crif"_fourcc:
            std::cout << "CRiff is found " << std::endl;
            break;
    }
}

godbolt example

Turtlefight
  • 9,420
  • 2
  • 23
  • 40
0

While fairly ugly you could cast your buffer to an integral type and then switch on that:

switch(*(std::uint32_t*)buffer)
{
case 'RIFF':
/* ... */
}

Note that this will very likely need careful management of endianness (not demonstrated here), either swapping the order of what is read or swapping the order of your case label values.

SoronelHaetir
  • 14,104
  • 1
  • 12
  • 23
  • 1
    I believe this violates the [strict aliasing rule](https://stackoverflow.com/q/98650/12149471). You can access any type as a sequence of characters, but not vice versa. – Andreas Wenzel Nov 24 '21 at 18:57
  • As @AndreasWenzel pointed out this violates the strict aliasing rule. You can use `std::bit_cast` in this case. The only time you're allowed to cast a char array to another type is if you actually constructed the type in the char array beforehand (with e.g. placement new) - but even then you need to use `std::launder` for the conversion since c++17. (see for example [this aligned_storage example](https://en.cppreference.com/w/cpp/types/aligned_storage) ) – Turtlefight Dec 01 '21 at 15:04