1

Right now, I'm working on an example code that I wish to integrate into my program later. What essentially I'm trying to do is read a .dat file byte by byte and interpret the data (ie. interpret boot sector to output the sector size, reserved sectors etc.)

To do this, I am reading the data byte by byte and, using the descriptions in fat12 of https://www.win.tue.nl/~aeb/linux/fs/fat/fat-1.html#ss1.3 , I translate the data into the information that I want. Right now, I can pull individual bytes from the file (Is it right to assume that the data pulled is in hex?). However, I need two bytes to have something meaningful. So, I need to combine two bytes into one, convert the hex data into decimal and output the information. Unfortunately, right now, I'm getting a seg fault and for the life of me, I can't figure out what's wrong. Thanks in advance!

int main (int argc, char **argv){

FILE *fp ,*fptest;
long lSize;
char *buffer;

//Open file
fptest= open("fat_volume.dat", "rb");

//Read file into buffer
fread(buffer,1,512,fptest);

//Parse the boot sector
char tmpA, tmpB;
tmpA = buffer[10]; //First byte
tmpB = buffer[11]; //Second byte

//Combine the two bytes into one
char combinedBytes[3];
strcpy (combinedBytes, tmpA);
strcat (combinedBytes, tmpB);

//Hex to decimal converter
long int li1;
li1 = strtol (combinedBytes,NULL,16);
printf ("The sector size is: %ld.\n", li1);

return 0;

}

user2989964
  • 131
  • 1
  • 8
  • 1
    You never `malloc` an array for `buffer`. Somewhere along the way, you need to say something like `buffer = malloc(512);` – cHao Nov 18 '13 at 23:36
  • Can you please explain? My C is incredibly rusty. An example would be greatly appreciated! – user2989964 Nov 18 '13 at 23:37
  • 3
    You define yourself a `char*`, but you never point it at anything. It's probably pointing to some unknown location that could open a wormhole if you try to access it. You need to set it to point to some memory you own. – cHao Nov 18 '13 at 23:40

2 Answers2

1

You must allocate buffer; e.g.

char buffer[512];

or

char *buffer = malloc(512);

EDIT:

The string operations

strcpy (combinedBytes, tmpA);
strcat (combinedBytes, tmpB);

do not make sense either and access/copy too much data (the compiler will warn you about this!).

I suggest do read values as

unsigned char tmpA = buffer[10];
unsigned char tmpB = buffer[11];

unsigned int tmp = (tmpA << 8) | (tmpB << 0);  /* or revert in in case of
                                                  little-endian */

To make things more efficient, I would write it as

struct fat_header {
    uint8_t pad0[10];
    uint16_t my_val;
    uint8_t pad1[500];
} __attribute__((__packed__));      /* this is not portable and for gcc! */

...

struct fat_header  hdr;

fread(&hdr, 1, sizeof hdr, f);
uint16_t val = be16toh(hdr.my_val); /* or le16toh() in case of le */
ensc
  • 6,704
  • 14
  • 22
  • Hi! I just tried your suggestion but I'm still getting a seg fault. Any ideas? – user2989964 Nov 18 '13 at 23:41
  • @user2989964: Among other things... Quit `str`-whatevering single chars. `char combinedBytes[] = { tmpA, tmpB, '\0' };` – cHao Nov 18 '13 at 23:46
  • Hi, I'm sorry for asking so many questions, but can you please explain more? I mainly program in Java, so I'm not really good with C. Thanks! – user2989964 Nov 18 '13 at 23:49
  • @user2989964: Basically... `char*` and `char` are two quite different things. You're treating a single char as if it were a string -- which, i imagine, is prompting the compiler to try and interpret that char as a pointer, which makes it go off the rails again. The compiler should have spit out a whole bunch of warnings about this stuff. Heed them. – cHao Nov 18 '13 at 23:59
  • C is not nearly as strict about types as Java is. Everything's just a number, and you need to be careful about what type of numbers you're giving it. – cHao Nov 19 '13 at 00:04
  • Thanks! I need time to digest all this. Is it okay to ask later if I have more questions? – user2989964 Nov 19 '13 at 00:08
  • @user2989964: As indicated in my answer: The compiler will warn you about the problems causing the segfaults (uninitialized `buffer`, bad parameter type). I recommend to build always with high warning level and to fix every diagnosed problem. – ensc Nov 19 '13 at 00:09
  • Just to check, if I want tmp to be little endian, it would be unsigned int tmp = (tmpA << 0) | (tmpB << 8); right? Thanks! – user2989964 Nov 19 '13 at 00:20
  • yes; that's correct for little endian input data (endianess of `tmp` does not matter here). – ensc Nov 19 '13 at 01:09
  • `tmpA << 0` is equal to `tmpA`, but requires a bit more mental parsing in order to determine it's not doing anything. `(tmpB << 8) | tmpA` or `(tmpA << 8) | tmpB` would be clearer in that regard, IMO. – cHao Nov 19 '13 at 15:45
  • @ensc: Not when `8` and `0` resemble each other, it doesn't. :P (And in the monospace font my browser's showing me right now, they do.) – cHao Nov 20 '13 at 06:32
0

You are reading into a buffer you never allocated memory for.

What you're trying now is to read from some junk value in memory, who knows, which almost always leads to a segmentation fault.

Use:

char *buffer = malloc(512 * sizeof(char)); // this allocates 512 times the size of a single char of memory

If you don't specify the number inside malloc to be of a specific size (e.g. malloc(512) the number is in bytes, though I think it's better to always include it.

This specific error is called dereferencing a null pointer

EDIT:

I've managed to run this code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main (int argc, char **argv)
{  
    FILE *fp ,*fptest;
    long lSize;
    char *buffer;

    //Open file
    fptest = fopen("input.txt", "rb");

    if (fptest == NULL)
    {
        printf("Error occurred when opening file");
        return 1;
    }

    buffer = malloc(sizeof(char) * 512);
    //Read file into buffer
    fread(buffer,1,512,fptest);

    //Parse the boot sector
    char tmpA, tmpB;
    tmpA = buffer[10]; //First byte
    tmpB = buffer[11]; //Second byte

    //Combine the two bytes into one
    char combinedBytes[3];
    strcpy (combinedBytes, &tmpA);
    strcat (combinedBytes, &tmpB);

    //Hex to decimal converter
    long int li1;
    li1 = strtol (combinedBytes,NULL,16);
    printf ("The sector size is: %ld.\n", li1);

    return 0;
}

You also used a function open() which must be fopen(), and you need to pass the address of tmpA and tmpB to strcpy and strcat.

This is why I don't understand why your compiler doesn't give any errors or warnings..

Community
  • 1
  • 1
edwardmp
  • 6,339
  • 5
  • 50
  • 77
  • 1
    No need for the `* sizeof(byte)`. A char is defined as one byte long. – cHao Nov 18 '13 at 23:43
  • Personally I think it's neater. – edwardmp Nov 18 '13 at 23:44
  • Hi! I tried your suggestion but I'm still getting a seg fault. Any ideas? Thank you BTW! – user2989964 Nov 18 '13 at 23:46
  • What's in the .dat file? When you try to access `buffer[10]` etc. are there even 11 characters in the file? I think `char combinedBytes[3]` is too small since it also has to carry the null terminator. Just try changing the `[3]` into something like `[10]` and see what happens. Try to use GDB to discover on which line it segfaults. – edwardmp Nov 18 '13 at 23:47
  • It's just a bunch of random data. What I'm trying access here is the boot sector of the .dat file. I'm learning file systems and I'm supposed to determine the sector size, hidden sectors etc. of a Fat12 .dat file. But I'm stuck in just parsing the file byte by byte. This is what I'm essentially trying to do with the code above. – user2989964 Nov 18 '13 at 23:53
  • Yes, but you are allocating three bytes, while you need at least 4. Because 2 characters + 2 `\0` special null characters makes 4 characters. So try `buffer[4]`. Do you know on which line it segfaults? – edwardmp Nov 18 '13 at 23:56
  • I'll try using gdb. Just a sec. – user2989964 Nov 18 '13 at 23:59
  • Thanks! I need time to digest all of this. Thanks again! – user2989964 Nov 19 '13 at 00:11
  • If you copy and paste my code, does it work? If yes please mark my answer as the one solving your problem. – edwardmp Nov 19 '13 at 00:13
  • It works, kinda. But it fixed my seg fault problem! How do I mark it as the solution? Thanks again! edit: NVM - found it! – user2989964 Nov 19 '13 at 00:23
  • @edwardmp: How are `strcpy` and `strcat` working with pointers to single chars? It looks like dumb luck; they both require a NUL to terminate the string. (Stack alignment may be inserting zeros. Seems a bad idea to rely on that.) You probably want `strncpy` and/or `strncat` instead. Or just say `combinedBytes[0] = tmpA; combinedBytes[1] = tmpB; combinedBytes[2] = '\0';`, since you know each thing being copied is one char. – cHao Nov 19 '13 at 15:49
  • @cHao yes, but I did not want to change his complete code, just make it work, though you are quite right in using ncpy and ncat – edwardmp Nov 19 '13 at 21:26