0

I'm having a difficult time trying to open a WAV file for reading. When I compile and run my code, I do not get any errors. I am adding the code that I am messing with (assuming necessary libraries have been called). The program is supposed to display the contents of the WAV file and although I enter a valid filename and extension, the statement "Invalid Filename. Try Again." is still being printed to the screen. The other method I attempted was enter the directory of the file instead of just the name, and when I do that, my program ends and nothing is displayed. Any guidance would be helpful and thank you in advance!

main(){
FILE *fin;
printf("\nEnter filename of WAV file: \n");
char filename[256];
scanf("%s",&filename);
fin = fopen(filename,"rb"); // opens in rb

if(!fin) // if file doesn't exist
{
printf("Invalid filename. Try again.\n");
}
else // if fin opens succesfully
{
    printf("\nFile opened succesfully\n");

    char *header;
    header = (char *)malloc(44);
    if(header == NULL)
    {
        printf("Error in allocating memory.");
        return 0;
    }


    fread(header,1,44,fin);

    char *chunkid;
    unsigned int *chunksize;
    char *format;
    char *subchunk1id;
    unsigned int *subchunk1size;
    unsigned short int *audioformat;
    unsigned short int *numchannels;
    unsigned int *samplerate;
    unsigned int *byterate;
    unsigned short int *blockalign;
    unsigned short int *bitspersample;
    char *subchunk2id;
    unsigned int *subchunk2size;
    unsigned int *data;

    chunkid = header;
    chunksize = (unsigned int *)(header + 4);
    format = header + 8;
    subchunk1id = header + 12;
    subchunk1size = (unsigned int *)(header + 16);
    audioformat = (unsigned short int *)(header + 18);
    numchannels = (unsigned short int*)(header + 20);
    samplerate = (unsigned int*)(header + 24);
    byterate = (unsigned int*)(header + 28);
    blockalign = (unsigned short int*)(header + 30);
    bitspersample = (unsigned short int*)(header + 32);
    subchunk2id = header + 36;
    subchunk2size = (unsigned int*)(header + 40);
    data = (unsigned int*)(header + 44);

    printf("\n%c%c%c%c",*(header),*(header+1),*(header+2),*(header+3));
    printf("\n%d",*chunksize);
    printf("\n%c%c%c%c",*(header + 8),*(header + 9), *(header + 10), *(header + 11));
    printf("\n%c%c%c%c",*(header + 12),*(header + 13), *(header + 14), *(header + 15));
    printf("\n%d",*subchunk1size);
    printf("\n%d",*audioformat);
    printf("\n%d",*numchannels);
    printf("\n%d",*samplerate);
    printf("\n%d",*byterate);
    printf("\n%d",*blockalign);
    printf("\n%d",*bitspersample);
    printf("\n%c%c%c%c",*(header + 36),*(header + 37),*(header + 38), *(header + 39));
    printf("\n%d",*subchunk2size);
    printf("\n%d",*data);


    fclose(fin);

} // end of else
} // end of main
Jay Juarez
  • 3
  • 2
  • You have at least two more closing braces (`}`) than open braces (`{`). – lurker Jul 07 '17 at 22:37
  • 1
    Not sure what compiler you are using, but `gcc` reports a warning on your `scanf` line: `warning: format ‘%s’ expects argument of type ‘char *’, but argument 2 has type ‘char (*)[256]’ [-Wformat=] scanf("%s", &filename);` – lurker Jul 07 '17 at 22:39
  • Oh, I get it. Change `scanf("%255s",&filename);` to `scanf("%255s",filename);`. You need to send the address of the string holder's first byte as an argument to `scanf()`, not a *pointer* to this address. – r3mainer Jul 07 '17 at 22:41
  • `numchannels = (unsigned short int*)(header + 20);` violates [strict aliasing](https://stackoverflow.com/questions/98650/what-is-the-strict-aliasing-rule). You can not be sure that `header + 20` is a valid address for an `unsigned short`. "It works on my system" does not change the fact that it's undefined behavior. – Andrew Henle Jul 07 '17 at 22:54
  • What operating system and compiler are you using? – edition Jul 08 '17 at 10:12
  • the function `main()` only has two valid signatures: `int main( void )` and `int main( int argc, char *argv[] )` Note they both have a return type of `int` any modern compiler (except visual studio) will issue a warning about the missing return type – user3629249 Jul 08 '17 at 23:47
  • for ease of readability and understanding: 1) consistently indent the code. indent after every opening brace '{'. unindent before every closing brace '}'. suggest each indent level be 4 spaces. – user3629249 Jul 08 '17 at 23:49
  • in C, the heap allocation functions: (malloc, calloc, realloc) 1) have return type `void*` which can be assigned to any pointer. Casting just clutters the code (and is error prone). – user3629249 Jul 08 '17 at 23:51
  • this kind of statement: `if(header == NULL)` is risky due to humans tend to make keypunch errors. like `if(header = NULL)`. The compiler will not catch this error. Suggest using: `if( !header )` or `if( NULL == header )` as such a keypunch error will be reported by the compiler, because cannot assign a value to a literal. – user3629249 Jul 08 '17 at 23:53
  • regarding: `fread(header,1,44,fin);` when calling system functions, like `fread()`, always check the returned value to assure the operation was successful. For the `fread()` function, the returned value should be the same as the third parameter. – user3629249 Jul 08 '17 at 23:55
  • the amount allocated for `*header` is only 44 bytes, so this line: `data = (unsigned int*)(header + 44);` is setting `data` to point one past the allocated memory buffer. I.E. outside the bounds of the `header` buffer. Dereferencing that address is undefined behavior – user3629249 Jul 09 '17 at 00:02
  • the posted code is making several assumptions. Those assumptions are not necessarily valid. Assumptions like: `unsigned int` is 4 bytes, `unsigned short int` is 2 bytes, etc. Strongly suggest the offsets be in terms of `sizeof( unsigned int )` and `sizeof( unsigned short int)` If you really want to be robust, define a struct with all the right fields and sizes and packed, then eliminate all those assignment statements to local variables. Note: the calls to `printf()` give absolutely no indication of the meaning of the fields being displayed on the terminal – user3629249 Jul 09 '17 at 00:08
  • the program has a memory leak because it failed to pass `header` to `free()`. Don't rely on the OS to cleanup after the waste land the program produces – user3629249 Jul 09 '17 at 00:09
  • the program sets several local variables, but does not use them. So those local variables (and the code to set them) can be eliminated – user3629249 Jul 09 '17 at 00:11
  • regarding: `scanf("%s",&filename);` 1) file name is an array and in C, the array name degrades to the address of the first byte of the array, therefore use: `scanf("%s",filename);` However, the input/format specifier: "%s" should always have a MAX CHARACTERS modifier that is one less than the length of the input buffer so the user cannot overflow the buffer. Such overflow is undefined behavior and can lead to a seg fault event. 3) when calling any of the `scanf()` family of functions, always check the returned value (not the parameter values) to assure the operation was successful. – user3629249 Jul 09 '17 at 00:15
  • the stream: `stdout` is buffered, so characters are not immediately displayed until a `fflush( stdout );` or the buffer overflows or a input operation is executed or the program exits or a newline sequence is output. When calling `printf()`, the format string should end with '\n' so the data is immediately output to the terminal. otherwise, as in the posted code: 1) each output is only displayed when the next call to `printf()` is executed 2) the last call to `printf()` is not displayed until the program exits. – user3629249 Jul 09 '17 at 00:19

2 Answers2

0

Try using

if(!fin)
{
  perror("Error:");
}

it will print the right error message for you to easily diagnose why your file couldn't be opened. May be your program cannot find the file specified by you or you might not have permissions to open it. Here's a list of errors you may encounter while opening a file.

The other possible problem that I could see is that you are using scanf to read from stdin into your char array. What if the path specified by you contains white-spaces? Every word followed by a White-space will be considered like a different string. And you'll get only the first longest possible word that doesn't contain white-space into it. So, your program isn't reading the entire path. You can try scanf("%[^\n]", filename) which will read until a '\n' is encountered. Beware! This exposes you to buffer overflow possibilities, that if you pass a valid string set to the program which is of length greater than 256, you'll be writing beyond the boundaries of your char array and which isn't good. So try scanf("%255[^\n]", filename) or try using fgets() doc SO which provides buffer overflow protection.

WhiteSword
  • 89
  • 9
0

Using warnings while compiling the code would have revealed your mistake.

Proposal for improvement, describe the header as a struct like:

typedef struct  
{
    char    riff_tag[4];
    int32_t riff_length;
    char    wave_tag[4];
    char    fmt_tag[4];
    int32_t fmt_length;
    int16_t audio_format;
    int16_t num_channels;
    int32_t sample_rate;
    int32_t byte_rate;
    int16_t block_align;
    int16_t bits_per_sample;
    char    data_tag[4];
    int32_t data_length;
}waveFileHeader, *waveFileHeaderptr ;

and read it like:

waveFileHeaderptr header =malloc ( sizeof (waveFileHeader));
fread (header , sizeof (waveFileHeader), 1 , fin);

will make the following code and handeling less messy